Replace should_stop by get_signaling_stream

Author: nicolas-grekas

docs/sidekicks.md

@@ -53,9 +53,29 @@ Each call **replaces** the entire snapshot atomically.
 - Throws `RuntimeException` if not called from a sidekick context
 - Throws `ValueError` if keys or values are not strings
 
-### `frankenphp_sidekick_should_stop(): bool`
+### `frankenphp_sidekick_get_signaling_stream(): resource`
 
-Returns `true` when FrankenPHP is shutting down.
+Returns a readable stream that becomes ready when FrankenPHP is shutting down or restarting workers.
+Use `stream_select()` instead of `sleep()` or `usleep()` to wait between iterations:
+
+```php
+function sidekick_should_stop(float $timeout = 0): bool
+{
+    static $signalingStream;
+    $signalingStream ??= frankenphp_sidekick_get_signaling_stream();
+    $s = (int) $timeout;
+
+    return 0 !== stream_select(...[[$signalingStream], [], [], $s, (int) (($timeout - $s) * 1e6)]);
+};
+
+do {
+    // ... do work, call set_vars() ...
+} while (!sidekick_should_stop(5));
+```
+
+**Why not `sleep()`?** `sleep()` and `usleep()` block at the C level and cannot be interrupted.
+A sidekick using `sleep(60)` would delay shutdown or worker restart by up to 60 seconds.
+`stream_select()` with the signaling stream wakes up immediately when FrankenPHP needs the thread to stop.
 
 - Throws `RuntimeException` if not called from a sidekick context
 
@@ -72,25 +92,29 @@ require __DIR__.'/../vendor/autoload.php';
 $command = $_SERVER['FRANKENPHP_SIDEKICK_NAME'] ?? '';
 
 match ($command) {
-    'redis-watcher' => runRedisWatcher(),
+    'redis-watcher' => run_redis_watcher(),
     default => throw new \RuntimeException("Unknown sidekick: $command"),
 };
 
-function runRedisWatcher(): void
+function sidekick_should_stop(float $timeout = 0): bool
+{
+    static $signalingStream;
+    $signalingStream ??= frankenphp_sidekick_get_signaling_stream();
+    $s = (int) $timeout;
+
+    return 0 !== stream_select(...[[$signalingStream], [], [], $s, (int) (($timeout - $s) * 1e6)]);
+};
+
+function run_redis_watcher(): void
 {
-    frankenphp_sidekick_set_vars([
-        'MASTER_HOST' => '10.0.0.1',
-        'MASTER_PORT' => '6379',
-    ]);
 
-    while (!frankenphp_sidekick_should_stop()) {
-        $master = discoverRedisMaster();
+    do {
+        $master = discover_redis_master();
         frankenphp_sidekick_set_vars([
             'MASTER_HOST' => $master['host'],
             'MASTER_PORT' => (string) $master['port'],
         ]);
-        usleep(100_000);
-    }
+    } while (!sidekick_should_stop(0.1));
 }
 ```
 
@@ -132,5 +156,5 @@ if (function_exists('frankenphp_sidekick_get_vars')) {
 - `SCRIPT_FILENAME` is set to the entrypoint's full path
 - `$_SERVER['FRANKENPHP_SIDEKICK_NAME']` and `$_SERVER['argv'][1]` contain the sidekick name
 - Crash recovery: automatic restart with exponential backoff
-- Graceful shutdown via `frankenphp_sidekick_should_stop()`
+- Graceful shutdown via `frankenphp_sidekick_get_signaling_stream()` and `stream_select()`
 - Use `error_log()` or `frankenphp_log()` for logging — avoid `echo`

frankenphp.c

@@ -86,6 +86,7 @@ HashTable *main_thread_env = NULL;
 __thread uintptr_t thread_index;
 __thread bool is_worker_thread = false;
 __thread char *sidekick_name = NULL;
+__thread int sidekick_stop_fds[2] = {-1, -1};
 __thread HashTable *sandboxed_env = NULL;
 
 void frankenphp_update_local_thread_context(bool is_worker) {
@@ -98,8 +99,16 @@ void frankenphp_update_local_thread_context(bool is_worker) {
 void frankenphp_set_sidekick_name(char *name) {
   sidekick_name = name;
   zend_unset_timeout();
+
+  /* Create a pipe for stop signaling */
+  if (pipe(sidekick_stop_fds) != 0) {
+    sidekick_stop_fds[0] = -1;
+    sidekick_stop_fds[1] = -1;
+  }
 }
 
+int frankenphp_sidekick_get_stop_fd_write(void) { return sidekick_stop_fds[1]; }
+
 static void frankenphp_update_request_context() {
   /* the server context is stored on the go side, still SG(server_context) needs
    * to not be NULL */
@@ -806,21 +815,41 @@ PHP_FUNCTION(frankenphp_sidekick_get_vars) {
   efree(vars_ptrs);
 }
 
-PHP_FUNCTION(frankenphp_sidekick_should_stop) {
+PHP_FUNCTION(frankenphp_sidekick_get_signaling_stream) {
   ZEND_PARSE_PARAMETERS_NONE();
 
-  int result = go_frankenphp_sidekick_should_stop(thread_index);
-  if (result < 0) {
-    zend_throw_exception(
-        spl_ce_RuntimeException,
-        "frankenphp_sidekick_should_stop() can only be called from a sidekick",
-        0);
+  if (sidekick_name == NULL) {
+    zend_throw_exception(spl_ce_RuntimeException,
+                         "frankenphp_sidekick_get_signaling_stream() can only "
+                         "be called from a sidekick",
+                         0);
     RETURN_THROWS();
   }
-  if (result > 0) {
-    RETURN_TRUE;
+
+  if (sidekick_stop_fds[0] < 0) {
+    zend_throw_exception(spl_ce_RuntimeException,
+                         "failed to create sidekick stop pipe", 0);
+    RETURN_THROWS();
   }
-  RETURN_FALSE;
+
+  /* Use php_stream_fopen_from_fd with dup'd fd so PHP can manage its own copy
+   */
+  int fd = dup(sidekick_stop_fds[0]);
+  if (fd < 0) {
+    zend_throw_exception(spl_ce_RuntimeException,
+                         "failed to dup sidekick stop fd", 0);
+    RETURN_THROWS();
+  }
+
+  php_stream *stream = php_stream_fopen_from_fd(fd, "rb", NULL);
+  if (!stream) {
+    close(fd);
+    zend_throw_exception(spl_ce_RuntimeException,
+                         "failed to create stream from stop fd", 0);
+    RETURN_THROWS();
+  }
+
+  php_stream_to_zval(stream, return_value);
 }
 
 PHP_FUNCTION(headers_send) {

frankenphp.h

@@ -172,6 +172,7 @@ bool frankenphp_shutdown_dummy_request(void);
 int frankenphp_execute_script(char *file_name);
 void frankenphp_update_local_thread_context(bool is_worker);
 void frankenphp_set_sidekick_name(char *name);
+int frankenphp_sidekick_get_stop_fd_write(void);
 
 int frankenphp_execute_script_cli(char *script, int argc, char **argv,
                                   bool eval);

frankenphp.stub.php

@@ -20,7 +20,8 @@ function frankenphp_sidekick_set_vars(array $vars): void {}
 
 function frankenphp_sidekick_get_vars(string|array $name, float $timeout = 30.0): array {}
 
-function frankenphp_sidekick_should_stop(): bool {}
+/** @return resource */
+function frankenphp_sidekick_get_signaling_stream() {}
 
 function headers_send(int $status = 200): int {}
 

frankenphp_arginfo.h

@@ -14,7 +14,7 @@ ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_frankenphp_sidekick_get_vars, 0,
 	ZEND_ARG_TYPE_INFO_WITH_DEFAULT_VALUE(0, timeout, IS_DOUBLE, 0, "30.0")
 ZEND_END_ARG_INFO()
 
-ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_frankenphp_sidekick_should_stop, 0, 0, _IS_BOOL, 0)
+ZEND_BEGIN_ARG_INFO_EX(arginfo_frankenphp_sidekick_get_signaling_stream, 0, 0, 0)
 ZEND_END_ARG_INFO()
 
 ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_headers_send, 0, 0, IS_LONG, 0)
@@ -57,7 +57,7 @@ ZEND_END_ARG_INFO()
 ZEND_FUNCTION(frankenphp_handle_request);
 ZEND_FUNCTION(frankenphp_sidekick_set_vars);
 ZEND_FUNCTION(frankenphp_sidekick_get_vars);
-ZEND_FUNCTION(frankenphp_sidekick_should_stop);
+ZEND_FUNCTION(frankenphp_sidekick_get_signaling_stream);
 ZEND_FUNCTION(headers_send);
 ZEND_FUNCTION(frankenphp_finish_request);
 ZEND_FUNCTION(frankenphp_request_headers);
@@ -70,7 +70,7 @@ static const zend_function_entry ext_functions[] = {
 	ZEND_FE(frankenphp_handle_request, arginfo_frankenphp_handle_request)
 	ZEND_FE(frankenphp_sidekick_set_vars, arginfo_frankenphp_sidekick_set_vars)
 	ZEND_FE(frankenphp_sidekick_get_vars, arginfo_frankenphp_sidekick_get_vars)
-	ZEND_FE(frankenphp_sidekick_should_stop, arginfo_frankenphp_sidekick_should_stop)
+	ZEND_FE(frankenphp_sidekick_get_signaling_stream, arginfo_frankenphp_sidekick_get_signaling_stream)
 	ZEND_FE(headers_send, arginfo_headers_send)
 	ZEND_FE(frankenphp_finish_request, arginfo_frankenphp_finish_request)
 	ZEND_FALIAS(fastcgi_finish_request, frankenphp_finish_request, arginfo_fastcgi_finish_request)

frankenphp_test.go

@@ -926,6 +926,35 @@ func TestSidekickCrashRestart(t *testing.T) {
 	})
 }
 
+func TestSidekickSignalingStream(t *testing.T) {
+	cwd, _ := os.Getwd()
+	entrypoint := cwd + "/testdata/sidekick-stop-fd-entrypoint.php"
+
+	runTest(t, func(handler func(http.ResponseWriter, *http.Request), _ *httptest.Server, _ int) {
+		body, _ := testGet("http://example.com/sidekick-stop-fd.php", handler, t)
+		assert.Equal(t, "stream", body)
+	}, &testOptions{
+		workerScript:       "sidekick-stop-fd.php",
+		nbWorkers:          1,
+		nbParallelRequests: 1,
+		initOpts:           []frankenphp.Option{frankenphp.WithMaxThreads(50)},
+		workerOpts: []frankenphp.WorkerOption{
+			frankenphp.WithWorkerSidekickRegistry(frankenphp.NewSidekickRegistry(entrypoint)),
+		},
+	})
+}
+
+func TestSidekickSignalingStreamNonSidekick(t *testing.T) {
+	runTest(t, func(handler func(http.ResponseWriter, *http.Request), _ *httptest.Server, _ int) {
+		body, _ := testGet("http://example.com/sidekick-stop-fd-non-sidekick.php", handler, t)
+		assert.Equal(t, "thrown", body)
+	}, &testOptions{
+		workerScript:       "sidekick-stop-fd-non-sidekick.php",
+		nbWorkers:          1,
+		nbParallelRequests: 1,
+	})
+}
+
 func ExampleServeHTTP() {
 	if err := frankenphp.Init(); err != nil {
 		panic(err)

phpthread.go

@@ -7,6 +7,7 @@ import (
 	"context"
 	"runtime"
 	"sync"
+	"syscall"
 	"unsafe"
 
 	"github.com/dunglas/frankenphp/internal/state"
@@ -71,6 +72,11 @@ func (thread *phpThread) shutdown() {
 		return
 	}
 
+	// Signal sidekick stop pipe to unblock stream_select/sleep
+	if handler, ok := thread.handler.(*workerThread); ok && !handler.worker.httpEnabled && handler.worker.sidekickStopFdWrite >= 0 {
+		syscall.Write(handler.worker.sidekickStopFdWrite, []byte{1})
+	}
+
 	close(thread.drainChan)
 	thread.state.WaitFor(state.Done)
 	thread.drainChan = make(chan struct{})

testdata/sidekick-binary-entrypoint.php

@@ -1,11 +1,12 @@
 <?php
 
+require __DIR__ . '/sidekick-helper.php';
+
 frankenphp_sidekick_set_vars([
     'BINARY_TEST' => "hello\x00world",
     'UTF8_TEST' => "héllo wörld 🚀",
     'EMPTY_VAL' => "",
 ]);
 
-while (!frankenphp_sidekick_should_stop()) {
-    usleep(10_000);
+while (!sidekick_should_stop(30)) {
 }

testdata/sidekick-crash.php

@@ -1,5 +1,7 @@
 <?php
 
+require __DIR__ . '/sidekick-helper.php';
+
 $marker = sys_get_temp_dir() . '/sidekick-crash-' . getmypid();
 $restarted = file_exists($marker);
 
@@ -10,8 +12,7 @@
 
 frankenphp_sidekick_set_vars(['SIDEKICK_STATUS' => 'restarted']);
 
-while (!frankenphp_sidekick_should_stop()) {
-    usleep(10_000);
+while (!sidekick_should_stop(30)) {
 }
 
 @unlink($marker);

testdata/sidekick-dedup.php

@@ -1,9 +1,10 @@
 <?php
 
+require __DIR__ . '/sidekick-helper.php';
+
 $name = $_SERVER['FRANKENPHP_SIDEKICK_NAME'] ?? $_SERVER['argv'][1] ?? 'unknown';
 
 frankenphp_sidekick_set_vars(['SIDEKICK_NAME' => $name]);
 
-while (!frankenphp_sidekick_should_stop()) {
-    usleep(10_000);
+while (!sidekick_should_stop(30)) {
 }