Skip to content

Commit 0d87765

Browse files
committed
call into cgo for reset directly, no fake dummy
1 parent df82e81 commit 0d87765

File tree

2 files changed

+43
-35
lines changed

2 files changed

+43
-35
lines changed

frankenphp.c

Lines changed: 35 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,25 @@ __thread HashTable *sandboxed_env = NULL;
8989
__thread zval *os_environment = NULL;
9090
zif_handler orig_opcache_reset;
9191

92+
/* Forward declaration */
93+
PHP_FUNCTION(frankenphp_opcache_reset);
94+
95+
/* Try to override opcache_reset if opcache is loaded.
96+
* Safe to call multiple times - skips if already overridden in this function
97+
* table. Uses handler comparison instead of orig_opcache_reset check so that
98+
* a fresh function table after PHP module restart is always re-overridden. */
99+
static void frankenphp_override_opcache_reset(void) {
100+
zend_function *func = zend_hash_str_find_ptr(CG(function_table), "opcache_reset",
101+
sizeof("opcache_reset") - 1);
102+
if (func != NULL && func->type == ZEND_INTERNAL_FUNCTION &&
103+
((zend_internal_function *)func)->handler !=
104+
ZEND_FN(frankenphp_opcache_reset)) {
105+
orig_opcache_reset = ((zend_internal_function *)func)->handler;
106+
((zend_internal_function *)func)->handler =
107+
ZEND_FN(frankenphp_opcache_reset);
108+
}
109+
}
110+
92111
void frankenphp_update_local_thread_context(bool is_worker) {
93112
is_worker_thread = is_worker;
94113

@@ -724,14 +743,8 @@ PHP_MINIT_FUNCTION(frankenphp) {
724743
php_error(E_WARNING, "Failed to find built-in getenv function");
725744
}
726745

727-
// Override opcache_reset
728-
func = zend_hash_str_find_ptr(CG(function_table), "opcache_reset",
729-
sizeof("opcache_reset") - 1);
730-
if (func != NULL && func->type == ZEND_INTERNAL_FUNCTION) {
731-
orig_opcache_reset = ((zend_internal_function *)func)->handler;
732-
((zend_internal_function *)func)->handler =
733-
ZEND_FN(frankenphp_opcache_reset);
734-
}
746+
// Override opcache_reset (may not be available yet if opcache loads after us)
747+
frankenphp_override_opcache_reset();
735748

736749
return SUCCESS;
737750
}
@@ -751,7 +764,14 @@ static zend_module_entry frankenphp_module = {
751764
static int frankenphp_startup(sapi_module_struct *sapi_module) {
752765
php_import_environment_variables = get_full_env;
753766

754-
return php_module_startup(sapi_module, &frankenphp_module);
767+
int result = php_module_startup(sapi_module, &frankenphp_module);
768+
if (result == SUCCESS) {
769+
/* All extensions are now loaded. Override opcache_reset if opcache
770+
* was not yet available during our MINIT (shared extension load order). */
771+
frankenphp_override_opcache_reset();
772+
}
773+
774+
return result;
755775
}
756776

757777
static int frankenphp_deactivate(void) { return SUCCESS; }
@@ -1412,17 +1432,12 @@ int frankenphp_execute_script_cli(char *script, int argc, char **argv,
14121432
}
14131433

14141434
int frankenphp_reset_opcache(void) {
1415-
zend_function *opcache_reset =
1416-
zend_hash_str_find_ptr(CG(function_table), ZEND_STRL("opcache_reset"));
1417-
if (opcache_reset) {
1418-
php_request_startup();
1419-
((zend_internal_function *)opcache_reset)->handler = orig_opcache_reset;
1420-
zend_call_known_function(opcache_reset, NULL, NULL, NULL, 0, NULL, NULL);
1421-
((zend_internal_function *)opcache_reset)->handler =
1422-
ZEND_FN(frankenphp_opcache_reset);
1423-
php_request_shutdown((void *)0);
1424-
}
1425-
1435+
zend_execute_data execute_data;
1436+
zval retval;
1437+
memset(&execute_data, 0, sizeof(execute_data));
1438+
ZVAL_UNDEF(&retval);
1439+
orig_opcache_reset(&execute_data, &retval);
1440+
zval_ptr_dtor(&retval);
14261441
return 0;
14271442
}
14281443

frankenphp.go

Lines changed: 8 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -765,6 +765,10 @@ func go_schedule_opcache_reset(threadIndex C.uintptr_t) {
765765
}
766766
}
767767

768+
// opcacheResetOnce ensures only one thread calls the actual opcache_reset.
769+
// Multiple threads calling it concurrently can race on shared memory.
770+
var opcacheResetOnce sync.Once
771+
768772
// restart all threads for an opcache_reset
769773
func restartThreadsAndOpcacheReset(withRegularThreads bool) {
770774
// disallow scaling threads while restarting workers
@@ -773,6 +777,7 @@ func restartThreadsAndOpcacheReset(withRegularThreads bool) {
773777

774778
threadsToRestart := drainThreads(withRegularThreads)
775779

780+
opcacheResetOnce = sync.Once{}
776781
opcacheResetWg := sync.WaitGroup{}
777782
for _, thread := range threadsToRestart {
778783
thread.state.Set(state.OpcacheResetting)
@@ -851,21 +856,9 @@ func drainThreads(withRegularThreads bool) []*phpThread {
851856
}
852857

853858
func scheduleOpcacheReset(thread *phpThread) {
854-
fc, _ := newDummyContext("/opcache_reset")
855-
856-
if workerThread, ok := thread.handler.(*workerThread); ok {
857-
workerThread.dummyFrankenPHPContext = fc
858-
defer func() { workerThread.dummyFrankenPHPContext = nil }()
859-
}
860-
861-
if regularThread, ok := thread.handler.(*regularThread); ok {
862-
regularThread.contextHolder.frankenPHPContext = fc
863-
defer func() { regularThread.contextHolder.frankenPHPContext = nil }()
864-
}
865-
866-
globalLogger.Info("resetting opcache", "thread", thread.name())
867-
C.frankenphp_reset_opcache()
868-
globalLogger.Info("opcache reset completed", "thread", thread.name())
859+
opcacheResetOnce.Do(func() {
860+
C.frankenphp_reset_opcache()
861+
})
869862
}
870863

871864
func convertArgs(args []string) (C.int, []*C.char) {

0 commit comments

Comments
 (0)