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


##########
src/iocore/net/SSLSecret.cc:
##########
@@ -34,6 +35,12 @@ namespace
 DbgCtl dbg_ctl_ssl_secret{"ssl_secret"};
 DbgCtl dbg_ctl_ssl_secret_err{"ssl_secret_err"};
 
+// Parallel ssl_multicert loading reaches loadSecret() from several 
std::threads
+// at once. Those are not EThreads, so a hook continuation's mutex does not
+// exclude them, and plugins written for one call at a time corrupt their own
+// state. This lock keeps the SSL_SECRET hooks to one caller at a time.
+std::mutex secret_hook_mutex;

Review Comment:
   This non-recursive mutex can deadlock TLS reloads with separate certificate 
and key files. After `SSLConfig` creates a fresh secret cache, the existing 
`ssl_secret_load_test` hook calls `TSSslSecretSet(cert)` before setting the 
key. That API synchronously calls `updateCTX()` → `update_ssl_ctx()`, which 
uses the previous certificate policies to call `getOrLoadSecret(cert, key)`. 
The missing key re-enters `loadSecret()` on the same thread and attempts to 
acquire the mutex it already holds, hanging the reload. Use a recursive mutex 
to allow same-thread re-entry while excluding other loader threads. Add a 
regression test that sets the certificate before its key during reload; the new 
counting-only hook does not exercise this path.



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