Skip to content

Commit f592e0f

Browse files
refactor: decouple worker threads from non-worker threads (#1137)
* Decouple workers. * Moves code to separate file. * Cleans up the exponential backoff. * Initial working implementation. * Refactors php threads to take callbacks. * Cleanup. * Cleanup. * Cleanup. * Cleanup. * Adjusts watcher logic. * Adjusts the watcher logic. * Fix opcache_reset race condition. * Fixing merge conflicts and formatting. * Prevents overlapping of TSRM reservation and script execution. * Adjustments as suggested by @dunglas. * Adds error assertions. * Adds comments. * Removes logs and explicitly compares to C.false. * Resets check. * Adds cast for safety. * Fixes waitgroup overflow. * Resolves waitgroup race condition on startup. * Moves worker request logic to worker.go. * Removes defer. * Removes call from go to c. * Fixes merge conflict. * Adds fibers test back in. * Refactors new thread loop approach. * Removes redundant check. * Adds compareAndSwap. * Refactor: removes global waitgroups and uses a 'thread state' abstraction instead. * Removes unnecessary method. * Updates comment. * Removes unnecessary booleans. * test * First state machine steps. * Splits threads. * Minimal working implementation with broken tests. * Fixes tests. * Refactoring. * Fixes merge conflicts. * Formatting * C formatting. * More cleanup. * Allows for clean state transitions. * Adds state tests. * Adds support for thread transitioning. * Fixes the testdata path. * Formatting. * Allows transitioning back to inactive state. * Fixes go linting. * Formatting. * Removes duplication. * Applies suggestions by @dunglas * Removes redundant check. * Locks the handler on restart. * Removes unnecessary log. * Changes Unpin() logic as suggested by @withinboredom * Adds suggestions by @dunglas and resolves TODO. * Makes restarts fully safe. * Will make the initial startup fail even if the watcher is enabled (as is currently the case) * Also adds compareAndSwap to the test. * Adds comment. * Prevents panic on initial watcher startup.
1 parent 2676bff commit f592e0f

20 files changed

+1076
-459
lines changed

cgi.go

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -227,8 +227,6 @@ func go_frankenphp_release_known_variable_keys(threadIndex C.uintptr_t) {
227227
for _, v := range thread.knownVariableKeys {
228228
C.frankenphp_release_zend_string(v)
229229
}
230-
// release everything that might still be pinned to the thread
231-
thread.Unpin()
232230
thread.knownVariableKeys = nil
233231
}
234232

frankenphp.c

Lines changed: 25 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,7 @@ static void frankenphp_free_request_context() {
8989
free(ctx->cookie_data);
9090
ctx->cookie_data = NULL;
9191

92-
/* Is freed via thread.Unpin() at the end of each request */
92+
/* Is freed via thread.Unpin() */
9393
SG(request_info).auth_password = NULL;
9494
SG(request_info).auth_user = NULL;
9595
SG(request_info).request_method = NULL;
@@ -243,7 +243,7 @@ PHP_FUNCTION(frankenphp_finish_request) { /* {{{ */
243243
php_header();
244244

245245
if (ctx->has_active_request) {
246-
go_frankenphp_finish_request(thread_index, false);
246+
go_frankenphp_finish_php_request(thread_index);
247247
}
248248

249249
ctx->finished = true;
@@ -443,7 +443,7 @@ PHP_FUNCTION(frankenphp_handle_request) {
443443

444444
frankenphp_worker_request_shutdown();
445445
ctx->has_active_request = false;
446-
go_frankenphp_finish_request(thread_index, true);
446+
go_frankenphp_finish_worker_request(thread_index);
447447

448448
RETURN_TRUE;
449449
}
@@ -811,9 +811,9 @@ static void set_thread_name(char *thread_name) {
811811
}
812812

813813
static void *php_thread(void *arg) {
814-
char thread_name[16] = {0};
815-
snprintf(thread_name, 16, "php-%" PRIxPTR, (uintptr_t)arg);
816814
thread_index = (uintptr_t)arg;
815+
char thread_name[16] = {0};
816+
snprintf(thread_name, 16, "php-%" PRIxPTR, thread_index);
817817
set_thread_name(thread_name);
818818

819819
#ifdef ZTS
@@ -832,7 +832,11 @@ static void *php_thread(void *arg) {
832832
cfg_get_string("filter.default", &default_filter);
833833
should_filter_var = default_filter != NULL;
834834

835-
while (go_handle_request(thread_index)) {
835+
// loop until Go signals to stop
836+
char *scriptName = NULL;
837+
while ((scriptName = go_frankenphp_before_script_execution(thread_index))) {
838+
go_frankenphp_after_script_execution(thread_index,
839+
frankenphp_execute_script(scriptName));
836840
}
837841

838842
go_frankenphp_release_known_variable_keys(thread_index);
@@ -841,6 +845,8 @@ static void *php_thread(void *arg) {
841845
ts_free_thread();
842846
#endif
843847

848+
go_frankenphp_on_thread_shutdown(thread_index);
849+
844850
return NULL;
845851
}
846852

@@ -858,13 +864,11 @@ static void *php_main(void *arg) {
858864
exit(EXIT_FAILURE);
859865
}
860866

861-
intptr_t num_threads = (intptr_t)arg;
862-
863867
set_thread_name("php-main");
864868

865869
#ifdef ZTS
866870
#if (PHP_VERSION_ID >= 80300)
867-
php_tsrm_startup_ex(num_threads);
871+
php_tsrm_startup_ex((intptr_t)arg);
868872
#else
869873
php_tsrm_startup();
870874
#endif
@@ -892,28 +896,7 @@ static void *php_main(void *arg) {
892896

893897
frankenphp_sapi_module.startup(&frankenphp_sapi_module);
894898

895-
pthread_t *threads = malloc(num_threads * sizeof(pthread_t));
896-
if (threads == NULL) {
897-
perror("malloc failed");
898-
exit(EXIT_FAILURE);
899-
}
900-
901-
for (uintptr_t i = 0; i < num_threads; i++) {
902-
if (pthread_create(&(*(threads + i)), NULL, &php_thread, (void *)i) != 0) {
903-
perror("failed to create PHP thread");
904-
free(threads);
905-
exit(EXIT_FAILURE);
906-
}
907-
}
908-
909-
for (int i = 0; i < num_threads; i++) {
910-
if (pthread_join((*(threads + i)), NULL) != 0) {
911-
perror("failed to join PHP thread");
912-
free(threads);
913-
exit(EXIT_FAILURE);
914-
}
915-
}
916-
free(threads);
899+
go_frankenphp_main_thread_is_ready();
917900

918901
/* channel closed, shutdown gracefully */
919902
frankenphp_sapi_module.shutdown(&frankenphp_sapi_module);
@@ -929,25 +912,30 @@ static void *php_main(void *arg) {
929912
frankenphp_sapi_module.ini_entries = NULL;
930913
}
931914
#endif
932-
933-
go_shutdown();
934-
915+
go_frankenphp_shutdown_main_thread();
935916
return NULL;
936917
}
937918

938-
int frankenphp_init(int num_threads) {
919+
int frankenphp_new_main_thread(int num_threads) {
939920
pthread_t thread;
940921

941922
if (pthread_create(&thread, NULL, &php_main, (void *)(intptr_t)num_threads) !=
942923
0) {
943-
go_shutdown();
944-
945924
return -1;
946925
}
947926

948927
return pthread_detach(thread);
949928
}
950929

930+
bool frankenphp_new_php_thread(uintptr_t thread_index) {
931+
pthread_t thread;
932+
if (pthread_create(&thread, NULL, &php_thread, (void *)thread_index) != 0) {
933+
return false;
934+
}
935+
pthread_detach(thread);
936+
return true;
937+
}
938+
951939
int frankenphp_request_startup() {
952940
if (php_request_startup() == SUCCESS) {
953941
return SUCCESS;
@@ -960,8 +948,6 @@ int frankenphp_request_startup() {
960948

961949
int frankenphp_execute_script(char *file_name) {
962950
if (frankenphp_request_startup() == FAILURE) {
963-
free(file_name);
964-
file_name = NULL;
965951

966952
return FAILURE;
967953
}
@@ -970,8 +956,6 @@ int frankenphp_execute_script(char *file_name) {
970956

971957
zend_file_handle file_handle;
972958
zend_stream_init_filename(&file_handle, file_name);
973-
free(file_name);
974-
file_name = NULL;
975959

976960
file_handle.primary_script = 1;
977961

0 commit comments

Comments
 (0)