Skip to content

Commit 505d966

Browse files
committed
async: scheduler slots are process-wide, registered once
The C slots are set once per process; per-thread state lives in the bridge's handlers container. The bridge no longer unregisters at RSHUTDOWN (it was yanking the slots from under other ZTS threads mid-request) and skips slot registration when its own module already owns them, so a later request just re-binds its handlers and launches. Core: register/unregister run under a mutex (two threads' first registrations raced the check-then-set), and the module name is copied into process memory - the bridge was passing a request-local string.
1 parent 538dd73 commit 505d966

2 files changed

Lines changed: 55 additions & 14 deletions

File tree

Zend/zend_async_API.c

Lines changed: 32 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -35,8 +35,12 @@ static void internal_globals_dtor(zend_async_globals_t *globals)
3535
(void) globals;
3636
}
3737

38+
static MUTEX_T scheduler_mutex = NULL;
39+
3840
void zend_async_globals_ctor(void)
3941
{
42+
scheduler_mutex = tsrm_mutex_alloc();
43+
4044
#ifdef ZTS
4145
ts_allocate_fast_id(&zend_async_globals_id, &zend_async_globals_offset,
4246
sizeof(zend_async_globals_t), (ts_allocate_ctor) internal_globals_ctor,
@@ -506,12 +510,17 @@ ZEND_API bool zend_async_scheduler_register(
506510
return false;
507511
}
508512

513+
tsrm_mutex_lock(scheduler_mutex);
514+
509515
/* A scheduler is registered once per process. */
510516
if (scheduler_module_name != NULL) {
517+
const char *owner = scheduler_module_name;
518+
519+
tsrm_mutex_unlock(scheduler_mutex);
511520
zend_error(E_CORE_WARNING,
512521
"The module %s cannot register an Async scheduler: %s already did",
513522
module,
514-
scheduler_module_name);
523+
owner);
515524
return false;
516525
}
517526

@@ -599,18 +608,28 @@ ZEND_API bool zend_async_scheduler_register(
599608
zend_async_coroutine_get_awaiting_info_fn = api->get_awaiting_info;
600609
}
601610

602-
scheduler_module_name = module;
611+
/* The caller's string may be request-local: keep a process copy. */
612+
scheduler_module_name = pestrdup(module, 1);
613+
614+
tsrm_mutex_unlock(scheduler_mutex);
615+
603616
ZEND_ASYNC_INITIALIZE;
604617

605618
return true;
606619
}
607620

608-
/* Withdraw the registration and reset every slot to its default. Safe per
609-
* request (the PHP bridge registers per request); the process-wide state —
610-
* the internal-context key registry — is NOT touched here. */
621+
/* Withdraw the registration and reset every slot to its default. The slots
622+
* are process-wide: this runs at process shutdown, or when a just-registered
623+
* scheduler fails to launch — never per request. The internal-context key
624+
* registry is NOT touched here. */
611625
ZEND_API void zend_async_scheduler_unregister(void)
612626
{
613-
scheduler_module_name = NULL;
627+
tsrm_mutex_lock(scheduler_mutex);
628+
629+
if (scheduler_module_name != NULL) {
630+
pefree((char *) scheduler_module_name, 1);
631+
scheduler_module_name = NULL;
632+
}
614633

615634
zend_async_new_coroutine_fn = NULL;
616635
zend_async_gc_new_coroutine_fn = NULL;
@@ -633,13 +652,20 @@ ZEND_API void zend_async_scheduler_unregister(void)
633652
zend_async_coroutine_add_awaiting_info_fn = NULL;
634653
zend_async_coroutine_remove_awaiting_info_fn = NULL;
635654
zend_async_coroutine_get_awaiting_info_fn = NULL;
655+
656+
tsrm_mutex_unlock(scheduler_mutex);
636657
}
637658

638659
void zend_async_api_shutdown(void)
639660
{
640661
zend_async_globals_dtor();
641662
zend_async_scheduler_unregister();
642663
internal_context_keys_shutdown();
664+
665+
if (scheduler_mutex != NULL) {
666+
tsrm_mutex_free(scheduler_mutex);
667+
scheduler_mutex = NULL;
668+
}
643669
}
644670

645671
ZEND_API bool zend_async_is_enabled(void)

ext/async_scheduler_hook/async_scheduler_hook.c

Lines changed: 23 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1110,18 +1110,37 @@ ZEND_METHOD(Async_SchedulerHook, register)
11101110
ASH_G(module) = zend_string_copy(module);
11111111
ASH_G(active) = true;
11121112

1113-
zend_async_scheduler_api_t api;
1114-
php_async_build_api(&api);
1113+
/* The C slots are process-wide and set once; per-thread state is the
1114+
* handlers container above. A later request (any thread) only re-binds
1115+
* its handlers and launches. */
1116+
const char *owner = zend_async_get_scheduler_module();
1117+
bool registered_now = false;
11151118

1116-
if (!zend_async_scheduler_register(ZSTR_VAL(module), &api)) {
1119+
if (owner == NULL) {
1120+
zend_async_scheduler_api_t api;
1121+
php_async_build_api(&api);
1122+
1123+
registered_now = zend_async_scheduler_register(ZSTR_VAL(module), &api);
1124+
1125+
if (!registered_now) {
1126+
/* Lost a registration race — acceptable if to ourselves. */
1127+
owner = zend_async_get_scheduler_module();
1128+
}
1129+
}
1130+
1131+
if (!registered_now && (owner == NULL || strcmp(owner, ZSTR_VAL(module)) != 0)) {
11171132
php_async_handlers_reset();
11181133
zend_throw_error(NULL, "Async\\SchedulerHook::register(): the engine refused the scheduler");
11191134
RETURN_THROWS();
11201135
}
11211136

11221137
if (!ZEND_ASYNC_SCHEDULER_LAUNCH()) {
11231138
php_async_handlers_reset();
1124-
zend_async_scheduler_unregister();
1139+
1140+
if (registered_now) {
1141+
zend_async_scheduler_unregister();
1142+
}
1143+
11251144
ZEND_ASYNC_DEACTIVATE;
11261145
RETURN_THROWS();
11271146
}
@@ -1372,10 +1391,6 @@ PHP_RSHUTDOWN_FUNCTION(async_scheduler_hook)
13721391
{
13731392
if (ASH_G(active)) {
13741393
php_async_handlers_reset();
1375-
1376-
/* The hooks died with this request; free the registration so the
1377-
* next request in this process can register a scheduler again. */
1378-
zend_async_scheduler_unregister();
13791394
ZEND_ASYNC_DEACTIVATE;
13801395
}
13811396

0 commit comments

Comments
 (0)