bneradt commented on PR #13556: URL: https://github.com/apache/trafficserver/pull/13556#issuecomment-5319927271
I think there is still a remap-state re-entry window at `unlockStates(remap_states)`. The lifecycle hook returns before `AutoStopCont` calls `shutdown_url_rewrite()` and `TSSystemState::shut_down_event_system()`. An existing `HttpSM` retains a `shared_ptr` lease on the remap configuration, so a remap callback already blocked on one of these mutexes can acquire it as soon as the barrier releases it and enter Lua after the global `__shutdown__` has freed shared process/FFI resources. That preserves the crash race when `proxy.config.plugin.dynamic_reload_mode=0`. The new AuTest drives shutdown load only through the global hook; the remap Lua script is exercised once before shutdown, so it does not detect this re-entry path. My suggested fix is to add a per-state execution-stop flag, for example `ts_lua_main_ctx::is_shutting_down`, protected by that state's mutex: 1. After the barrier has acquired every global and remap state mutex, but before invoking any `__shutdown__`, set `is_shutting_down = true` on every state. 2. In every runtime Lua entry/resume path, check the flag immediately after acquiring the state mutex. If it is set, skip Lua and perform the appropriate ATS-side reenable/cleanup. This includes the global/remap/vconn handlers and coroutine, transform, intercept, and fetch resume paths. 3. Keep explicit teardown such as `ts_lua_del_module()` exempt from the runtime gate, so `TSRemapDeleteInstance` can still acquire the released remap mutexes and invoke `__clean__`. 4. The global locks can remain held as this patch already does; the remap locks can then be released safely because queued callbacks will acquire them, observe the flag, and decline to enter Lua. Because the flag is set only while the barrier owns all state mutexes and is read only after acquiring the corresponding mutex, it does not need to be atomic: callbacks that were already executing drain before the barrier completes, while callbacks queued behind it see the stopped state. I would also extend the AuTest with a remap Lua callback held/queued across SIGTERM and assert that it never logs execution after the global shutdown marker. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
