Off-thread TLS reenable can contend with owner-thread NetHandler queue operations

[moonchen]

TSVConnReenableEx() can acquire a connection's NetHandler mutex on a task thread. The connection's own network thread can then fail to acquire that mutex and hit an active/keep-alive queue assertion, even though it is running on the correct thread.

Certifier uses this API from a task thread. This is a possible explanation for #13358, but we have not confirmed the cause of that crash.

What happens

1. A TLS connection belongs to network thread A.
2. A plugin calls TSVConnReenable() from task thread B.
3. ATS tries to acquire A's NetHandler mutex on B. If successful, it runs TLS reenable on B. It schedules work back to A only if the lock attempt fails.
4. Meanwhile, A can run a queued HTTP/cache callback without holding its NetHandler mutex.
5. If that callback reaches an active/keep-alive queue operation while B holds the mutex, the queue's try-lock fails. Those queue operations assert on failure.

The two operations can involve different connections sharing the same NetHandler. No connection migration is needed.

Reproduction

A diagnostic plugin registers two TS_SSL_CERT_HOOK callbacks:

1. The first pauses the handshake and schedules a task callback to call TSVConnReenable().
2. A separate callback runs on the owner thread with its own mutex. The task callback waits until this owner callback is running before reenabling TLS.
3. The second TLS hook runs on the task thread while holding the NetHandler mutex. It waits briefly while the owner callback tries the same mutex.

Observed on an ATS 11.0.0 debug build; addresses are replaced with A/B:

tls second owner=A current=B task_thread=1 holds_nh=1
tls owner=A current=A nh_owner=A holder=B trylock=0

Confirmed: the task thread holds the NetHandler mutex while the owner thread fails to acquire it. No ATS core changes were needed.

Not yet reproduced: the complete certifier/cache workload or queue assertion from #13358. The probe widens the timing window and checks the failed try-lock without invoking the queue operation. The same relevant code exists in 10.1.2, but that version was not run.

Agreed fix direction

After discussing this with community members, we agreed that work protected by a NetHandler mutex should run on that NetHandler's owning network thread. We prefer to move the work to the owner before taking the mutex, rather than let another thread acquire it.

• Caller is on another thread: schedule the TLS callback on getThreadForTLSEvents() before attempting to acquire the NetHandler mutex. Scheduling on an arbitrary ET_NET thread is not sufficient.
• Caller is already on the owner: preserve synchronous execution with the required locks and the existing lock-failure fallback. In particular, TLS verification hooks need their error result to take effect before the verification callback returns.
• Task-thread callers remain supported: calling this API from another thread is documented as safe. ATS should handle the return to the owner; certifier does not need to move its certificate generation onto a network thread.

This is the preferred direction for the fix, not an invariant the current code already enforces. Other acquisition paths, including UnixNetVConnection::reenable(VIO*) and callbacks carrying an NH mutex, need the same ownership audit. Keep existing try-lock failure handling while those paths are addressed.

Moving only teardown to the owner would not solve this case: teardown can already be on the correct thread. The API change prevents the demonstrated foreign-thread acquisition; we have not yet confirmed that this is the cause of #13358.

Source references and history

• TSVConnReenableEx() takes the mutex on the calling thread. For SSL connections, getMutexForTLSEvents() returns nh->mutex.
• EThread::process_event() takes the event's mutex, which need not be the NetHandler mutex.
• The active/keep-alive queue helpers assert when their NetHandler try-lock fails.
• Certifier's shadow_cert_generator() reenables TLS connections from a task thread.
• NetVC maybe lost from read_ready_list if Plugin call TSVConnReenable() #1546 deliberately allowed TLS reenable on another thread under the NetHandler lock. The agreed direction above changes that approach. Assert if operations on keep_alive_queue and active_queue are not thread-safe #4379 added the queue assertions. Other paths, such as reenable(VIO*), still permit acquisition from another thread and need auditing.

Found while investigating #13358 and #13362. The separate session reenable issue is #13744.