moonchen opened a new issue, #13744: URL: https://github.com/apache/trafficserver/issues/13744
`TSHttpSsnReenable()` checks whether its caller belongs to `ET_NET`, but its successful try-lock path does not check whether the caller is the session's affinity thread. An asynchronous session hook that reenables from a different network thread can therefore dispatch subsequent session hooks and session processing on that thread. ## Relevant path In [`TSHttpSsnReenable`](https://github.com/apache/trafficserver/blob/a88ba2095384846454fb04ae745cb718663cdc11/src/api/InkAPI.cc#L3891): ```text Session-start hook pauses on session owner A -> async completion on another ET_NET thread B -> TSHttpSsnReenable(session, TS_EVENT_HTTP_CONTINUE) -> B passes is_event_type(ET_NET) -> B acquires the session mutex -> session->handleEvent(...) runs inline on B ``` The non-ET_NET and failed-lock branches already use session affinity when available. The successful-lock branch bypasses that scheduling. In contrast, [`TSHttpTxnReenable`](https://github.com/apache/trafficserver/blob/a88ba2095384846454fb04ae745cb718663cdc11/src/api/InkAPI.cc#L5220) checks the exact state-machine affinity thread. ## Reproduction With at least two network threads, register two `TS_HTTP_SSN_START_HOOK` callbacks: 1. The first records its owner thread A, saves the session, and returns without reenabling it. 2. Schedule a separate continuation on another network thread B using `TSContScheduleOnThread()`. After the first callback returns, call `TSHttpSsnReenable()` from B. 3. Record the executing thread in the second session-start hook. An ATS 11.0.0 debug build produced the following output, with addresses replaced by A/B: ```text session first owner=A other=B session resume owner=A current=B session second owner=A current=B wrong_thread=1 ``` No core modifications were needed. The second hook deliberately scheduled the final reenable back onto A, so this probe demonstrates wrong-thread hook dispatch without proceeding into wrong-thread session startup. The same API branch exists in 10.1.2; that release was inspected, not run. ## Consequences and expected behavior After the start hooks complete, [`ProxySession::handle_api_return`](https://github.com/apache/trafficserver/blob/a88ba2095384846454fb04ae745cb718663cdc11/src/proxy/ProxySession.cc#L158) calls `start()` synchronously. [`Http2ClientSession::start`](https://github.com/apache/trafficserver/blob/a88ba2095384846454fb04ae745cb718663cdc11/src/proxy/http2/Http2ClientSession.cc#L76) sets up I/O and can process prebuffered input immediately. That creates a potential route into connection processing on the wrong thread. This consequence is based on source tracing; an HTTP/2 crash was not reproduced. Reenable from a different ET_NET thread should dispatch through the session's owner with the required locks, as the existing non-ET_NET path does. Being in the same thread pool and acquiring the session mutex is insufficient to establish connection ownership. Related history: #7255 and its fix #7295 addressed delayed session-start reenable from non-ET_NET threads. The successful-lock branch for a different ET_NET caller remains uncovered. A delayed callback on `TS_THREAD_POOL_TASK` does not exercise this branch. This was found while investigating #13358/#13362, but there is no evidence yet that the reported deployment uses a session hook that reenables this way. The shipped callers examined reenable synchronously; certifier uses `TSVConnReenable()`, not this API. This should be tracked as an independent affinity defect rather than an established cause of that report. -- 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]
