Copilot commented on code in PR #13045:
URL: https://github.com/apache/trafficserver/pull/13045#discussion_r3088729371


##########
plugins/lua/ts_lua.cc:
##########
@@ -827,6 +827,47 @@ globalHookHandler(TSCont contp, TSEvent event ATS_UNUSED, 
void *edata)
   return 0;
 }
 
+static int
+shutdownHookHandler(TSCont contp, TSEvent /* event ATS_UNUSED */, void * /* 
edata ATS_UNUSED */)
+{
+  ts_lua_instance_conf *const conf = (ts_lua_instance_conf 
*)TSContDataGet(contp);
+
+  for (int index = 0; index < conf->states; ++index) {
+    ts_lua_main_ctx *const main_ctx = &ts_lua_g_main_ctx_array[index];
+
+    TSMutexLock(main_ctx->mutexp);

Review Comment:
   `TS_LIFECYCLE_SHUTDOWN_HOOK` is invoked from `traffic_server.cc` just before 
`TSSystemState::shut_down_event_system()` (i.e., other event threads may still 
be running). Using blocking `TSMutexLock(main_ctx->mutexp)` here risks stalling 
shutdown if any Lua state mutex is held by in-flight work. Consider using 
`TSMutexLockTry()` (and skipping/logging when the lock can't be acquired), or 
otherwise ensuring the hook can't block ATS shutdown.
   ```suggestion
       if (TSMutexLockTry(main_ctx->mutexp) != TS_SUCCESS) {
         TSError("[ts_lua][%s] skipping shutdown callback for script '%s' state 
%d because the lua state is busy",
                 __FUNCTION__, conf->script, index);
         continue;
       }
   ```



-- 
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]

Reply via email to