Skip to content

Commit 73ba679

Browse files
Merge branches 'force-kill-primitives', 'persistent-zval-helpers' and 'thread-handler-drain-seam' into bg-worker-integration
* persistent-zval-helpers: feat: persistent-zval helpers (deep-copy zval trees across threads) * thread-handler-drain-seam: refactor: add drain() seam to threadHandler interface
3 parents 34e1a36 + 0ab3940 + d848515 commit 73ba679

12 files changed

Lines changed: 483 additions & 4 deletions

.github/workflows/sanitizers.yaml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -108,7 +108,7 @@ jobs:
108108
- name: Set CGO flags
109109
run: |
110110
{
111-
echo "CGO_CFLAGS=$CFLAGS -I${PWD}/watcher/target/include $(php-config --includes)"
111+
echo "CGO_CFLAGS=$CFLAGS -I${PWD}/watcher/target/include -DFRANKENPHP_TEST $(php-config --includes)"
112112
echo "CGO_LDFLAGS=$LDFLAGS $(php-config --ldflags) $(php-config --libs)"
113113
} >> "$GITHUB_ENV"
114114
- name: Run tests

.github/workflows/tests.yaml

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ jobs:
5959
- name: Install e-dant/watcher
6060
uses: ./.github/actions/watcher
6161
- name: Set CGO flags
62-
run: echo "CGO_CFLAGS=-I${PWD}/watcher/target/include $(php-config --includes)" >> "${GITHUB_ENV}"
62+
run: echo "CGO_CFLAGS=-I${PWD}/watcher/target/include -DFRANKENPHP_TEST $(php-config --includes)" >> "${GITHUB_ENV}"
6363
- name: Build
6464
run: go build
6565
- name: Build testcli binary
@@ -135,7 +135,7 @@ jobs:
135135
echo "GEN_STUB_SCRIPT=${PWD}/php-${PHP_VERSION}/build/gen_stub.php" >> "${GITHUB_ENV}"
136136
- name: Set CGO flags
137137
run: |
138-
echo "CGO_CFLAGS=$(php-config --includes)" >> "${GITHUB_ENV}"
138+
echo "CGO_CFLAGS=-DFRANKENPHP_TEST $(php-config --includes)" >> "${GITHUB_ENV}"
139139
echo "CGO_LDFLAGS=$(php-config --ldflags) $(php-config --libs)" >> "${GITHUB_ENV}"
140140
- name: Install gotestsum
141141
run: go install gotest.tools/gotestsum@latest
@@ -172,7 +172,7 @@ jobs:
172172
- name: Set CGO flags
173173
run: |
174174
{
175-
echo "CGO_CFLAGS=-I/opt/homebrew/include/ $(php-config --includes)"
175+
echo "CGO_CFLAGS=-I/opt/homebrew/include/ -DFRANKENPHP_TEST $(php-config --includes)"
176176
echo "CGO_LDFLAGS=-L/opt/homebrew/lib/ $(php-config --ldflags) $(php-config --libs)"
177177
} >> "${GITHUB_ENV}"
178178
- name: Build

frankenphp.c

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,12 @@
3737

3838
#include "_cgo_export.h"
3939
#include "frankenphp_arginfo.h"
40+
#ifdef FRANKENPHP_TEST
41+
/* The persistent_zval helpers are only compiled in when a consumer needs
42+
* them. The step that lands the first real caller (background workers)
43+
* will drop this guard. */
44+
#include "zval.h"
45+
#endif
4046

4147
#if defined(PHP_WIN32) && defined(ZTS)
4248
ZEND_TSRMLS_CACHE_DEFINE()
@@ -795,12 +801,55 @@ PHP_FUNCTION(frankenphp_log) {
795801
}
796802
}
797803

804+
#ifdef FRANKENPHP_TEST
805+
/* Test-only entry point that exercises zval.h end-to-end:
806+
* validate -> persist (request -> persistent memory) ->
807+
* to_request (persistent -> fresh request memory) -> free persistent copy.
808+
* Compiled only when FRANKENPHP_TEST is defined; never registered
809+
* in production builds. */
810+
ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(
811+
arginfo_frankenphp_test_persist_roundtrip, 0, 1, IS_MIXED, 0)
812+
ZEND_ARG_TYPE_INFO(0, value, IS_MIXED, 0)
813+
ZEND_END_ARG_INFO()
814+
815+
PHP_FUNCTION(frankenphp_test_persist_roundtrip) {
816+
zval *input;
817+
ZEND_PARSE_PARAMETERS_START(1, 1)
818+
Z_PARAM_ZVAL(input)
819+
ZEND_PARSE_PARAMETERS_END();
820+
821+
if (!persistent_zval_validate(input)) {
822+
zend_throw_exception(spl_ce_LogicException,
823+
"persistent_zval: value type not supported "
824+
"(only scalars, arrays, and enums are allowed)",
825+
0);
826+
RETURN_THROWS();
827+
}
828+
829+
zval persistent;
830+
persistent_zval_persist(&persistent, input);
831+
persistent_zval_to_request(return_value, &persistent);
832+
persistent_zval_free(&persistent);
833+
}
834+
835+
static const zend_function_entry frankenphp_test_hook_functions[] = {
836+
PHP_FE(frankenphp_test_persist_roundtrip,
837+
arginfo_frankenphp_test_persist_roundtrip) PHP_FE_END};
838+
#endif
839+
798840
PHP_MINIT_FUNCTION(frankenphp) {
799841
register_frankenphp_symbols(module_number);
800842
#ifndef PHP_WIN32
801843
pthread_atfork(NULL, NULL, frankenphp_fork_child);
802844
#endif
803845

846+
#ifdef FRANKENPHP_TEST
847+
if (zend_register_functions(NULL, frankenphp_test_hook_functions, NULL,
848+
MODULE_PERSISTENT) == FAILURE) {
849+
return FAILURE;
850+
}
851+
#endif
852+
804853
zend_function *func;
805854

806855
// Override putenv

phpthread.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,12 @@ type threadHandler interface {
4141
afterScriptExecution(exitStatus int)
4242
context() context.Context
4343
frankenPHPContext() *frankenPHPContext
44+
// drain is a hook called by drainWorkerThreads right before drainChan is
45+
// closed. Handlers that need to wake up a thread parked in a blocking C
46+
// call (e.g. by closing a stop pipe) plug their signal in here. All
47+
// current handlers are no-ops; this is the seam later handler types use
48+
// without having to modify drainWorkerThreads.
49+
drain()
4450
}
4551

4652
func newPHPThread(threadIndex int) *phpThread {

testdata/persist-roundtrip.php

Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,88 @@
1+
<?php
2+
3+
// Exercises frankenphp_test_persist_roundtrip (only registered when
4+
// FRANKENPHP_TEST_HOOKS is defined at build time). The script runs as a
5+
// plain non-worker request; the Go harness asserts the combined output.
6+
7+
enum Status: string {
8+
case Active = 'active';
9+
case Paused = 'paused';
10+
}
11+
12+
function same(mixed $actual, mixed $expected, string $label): void {
13+
if ($actual !== $expected) {
14+
echo "FAIL $label: expected ";
15+
var_export($expected);
16+
echo ' got ';
17+
var_export($actual);
18+
echo "\n";
19+
return;
20+
}
21+
echo "OK $label\n";
22+
}
23+
24+
$rt = 'frankenphp_test_persist_roundtrip';
25+
if (!function_exists($rt)) {
26+
echo "SKIP frankenphp_test_persist_roundtrip not registered\n";
27+
return;
28+
}
29+
30+
// Scalars.
31+
same($rt(null), null, 'null');
32+
same($rt(false), false, 'false');
33+
same($rt(true), true, 'true');
34+
same($rt(0), 0, 'int zero');
35+
same($rt(42), 42, 'int');
36+
same($rt(-1), -1, 'int negative');
37+
same($rt(1.5), 1.5, 'float');
38+
same($rt(''), '', 'empty string');
39+
same($rt('hello'), 'hello', 'short string');
40+
41+
// Long (non-interned) string: forces the allocation path.
42+
$long = str_repeat('x', 1024);
43+
same($rt($long), $long, 'long string');
44+
45+
// Nested arrays, mixed keys.
46+
$arr = [
47+
'name' => 'alice',
48+
'age' => 30,
49+
'tags' => ['admin', 'editor'],
50+
'meta' => ['created' => 1234567890, 'flags' => [true, false, null]],
51+
0 => 'first',
52+
1 => 'second',
53+
];
54+
same($rt($arr), $arr, 'nested array');
55+
56+
// Enum roundtrip: identity (===) must be preserved because the enum is
57+
// re-resolved to the same singleton case on the read side.
58+
same($rt(Status::Active), Status::Active, 'enum active');
59+
same($rt(Status::Paused), Status::Paused, 'enum paused');
60+
61+
// Array containing an enum.
62+
same(
63+
$rt(['status' => Status::Active, 'count' => 7]),
64+
['status' => Status::Active, 'count' => 7],
65+
'array with enum',
66+
);
67+
68+
// Invalid inputs throw LogicException.
69+
try {
70+
$rt(new stdClass());
71+
echo "FAIL stdClass should throw\n";
72+
} catch (\LogicException) {
73+
echo "OK stdClass rejected\n";
74+
}
75+
76+
try {
77+
$rt(fopen('php://memory', 'r'));
78+
echo "FAIL resource should throw\n";
79+
} catch (\LogicException) {
80+
echo "OK resource rejected\n";
81+
}
82+
83+
try {
84+
$rt(['ok' => 1, 'bad' => new stdClass()]);
85+
echo "FAIL nested stdClass should throw\n";
86+
} catch (\LogicException) {
87+
echo "OK nested stdClass rejected\n";
88+
}

threadinactive.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,3 +58,5 @@ func (handler *inactiveThread) context() context.Context {
5858
func (handler *inactiveThread) name() string {
5959
return "Inactive PHP Thread"
6060
}
61+
62+
func (handler *inactiveThread) drain() {}

threadregular.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,8 @@ func (handler *regularThread) name() string {
8383
return "Regular PHP Thread"
8484
}
8585

86+
func (handler *regularThread) drain() {}
87+
8688
func (handler *regularThread) waitForRequest() string {
8789
// max_requests reached: restart the thread to clean up all ZTS state
8890
if maxRequestsPerThread > 0 && handler.requestCount >= maxRequestsPerThread {

threadtasks_test.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,8 @@ func (handler *taskThread) name() string {
7979
return "Task PHP Thread"
8080
}
8181

82+
func (handler *taskThread) drain() {}
83+
8284
func (handler *taskThread) waitForTasks() {
8385
for {
8486
select {

threadworker.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -105,6 +105,8 @@ func (handler *workerThread) name() string {
105105
return "Worker PHP Thread - " + handler.worker.fileName
106106
}
107107

108+
func (handler *workerThread) drain() {}
109+
108110
func setupWorkerScript(handler *workerThread, worker *worker) {
109111
metrics.StartWorker(worker.name)
110112

worker.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -191,6 +191,7 @@ func drainWorkerThreads() (drainedThreads []*phpThread) {
191191
continue
192192
}
193193

194+
thread.handler.drain()
194195
close(thread.drainChan)
195196
drainedThreads = append(drainedThreads, thread)
196197

0 commit comments

Comments
 (0)