moonchen opened a new issue, #13743: URL: https://github.com/apache/trafficserver/issues/13743
`TSVConnReenableEx()` can acquire a connection's NetHandler mutex on a task thread and run TLS reenable there. At the same time, the NetHandler's owner thread can run a queued HTTP/cache continuation without holding that mutex. If that continuation reaches an active/keep-alive queue operation, its try-lock can fail even though it is on the correct thread; those operations currently assert on failure. This is a candidate explanation for #13358, investigated while reviewing #13362. The conflicting lock condition was reproduced with a diagnostic plugin. The complete reported cache/HTTP workload and crash have not been reproduced, and the actual mutex holder in the reporter's process remains unidentified. ## Relevant paths - [`TSVConnReenableEx`](https://github.com/apache/trafficserver/blob/a88ba2095384846454fb04ae745cb718663cdc11/src/api/InkAPI.cc#L8487) tries the TLS-event mutex on the calling thread and calls `reenable()` inline if successful. Only failure schedules the callback on the owner. - For an SSL connection, [`getMutexForTLSEvents`](https://github.com/apache/trafficserver/blob/a88ba2095384846454fb04ae745cb718663cdc11/src/iocore/net/SSLNetVConnection.cc#L1785) returns `nh->mutex`. `reenable()` can invoke subsequent TLS hooks and update the NetHandler ready lists while holding it. - [`EThread::process_event`](https://github.com/apache/trafficserver/blob/a88ba2095384846454fb04ae745cb718663cdc11/src/iocore/eventsystem/UnixEThread.cc#L153) acquires the queued continuation's mutex, which need not be `nh->mutex`. - [`UnixNetVConnection::add_to_keep_alive_queue`](https://github.com/apache/trafficserver/blob/a88ba2095384846454fb04ae745cb718663cdc11/src/iocore/net/UnixNetVConnection.cc#L1367) and the adjacent active/keep-alive queue methods assert if their NetHandler try-lock fails. The TLS operation and queue operation can concern different connections sharing the same NetHandler. Connection migration is not required. ## Diagnostic reproduction Using an ATS 11.0.0 debug build, register two `TS_SSL_CERT_HOOK` callbacks: 1. The first pauses the handshake on owner thread A and schedules a callback on `TS_THREAD_POOL_TASK` to call `TSVConnReenable()`. 2. Queue a separate continuation on A with its own mutex, so A can execute it outside the NetHandler poll handler. 3. Have the task callback wait until that owner continuation is running, then call `TSVConnReenable()`. 4. The second certificate hook runs inline on task thread B, holding A's NetHandler mutex. Use bounded synchronization to keep it there while the owner continuation tries the same mutex. Observed output, with thread addresses replaced by A/B: ```text tls second owner=A current=B task_thread=1 holds_nh=1 tls owner=A current=A nh_owner=A holder=B trylock=0 ``` The diagnostic plugin widens the interleaving; it does not modify the ATS core or invoke the queue assertion. This demonstrates the lock conflict, not a full certifier/cache crash reproduction. The relevant API and queue branches also exist in 10.1.2, which was inspected but not run for this probe. ## Relevance and expected behavior The reported deployment uses certifier. Its [`shadow_cert_generator`](https://github.com/apache/trafficserver/blob/a88ba2095384846454fb04ae745cb718663cdc11/plugins/certifier/certifier.cc#L547) reenables queued TLS connections from a task thread. Calling `TSVConnReenable()` from another thread is explicitly supported by its [documentation](https://github.com/apache/trafficserver/blob/a88ba2095384846454fb04ae745cb718663cdc11/doc/developer-guide/api/functions/TSVConnReenable.en.rst#L51). The API call itself is not plugin misuse. The API implementation and NetHandler queue operations need compatible locking/threading assumptions so a supported async reenable cannot cause an owner-thread queue assertion. Routing off-owner TLS reenables home before acquiring the NetHandler mutex is one possible approach; other foreign holders and callers that arrive without the mutex need auditing. Historical context: #1546 deliberately allowed off-owner reenable while holding the NetHandler mutex to protect ready-list updates. The issue here is the conflict with queue callers that assume a try-lock must succeed, not simply the existence of an off-thread TLS callback. An owner-thread check or teardown bounce alone does not resolve this interleaving. Contention is explicitly handled elsewhere: [`UnixNetVConnection::reenable(VIO*)`](https://github.com/apache/trafficserver/blob/a88ba2095384846454fb04ae745cb718663cdc11/src/iocore/net/UnixNetVConnection.cc#L371) falls back to atomic enable queues and signals the owner if its NetHandler try-lock fails. Thus the queue assertions do not establish a universal owner-only or contention-free mutex contract. #4379 introduced these assertions while discussing both thread affinity and the need to hold the NetHandler lock. The fix needs to reconcile those requirements at the affected entry points; the probe alone does not decide which threading policy ATS should enforce globally. -- 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]
