moonchen opened a new pull request, #13406: URL: https://github.com/apache/trafficserver/pull/13406
When an `sni` rule has a `limit` and a queue, connections over the limit are parked in a queue at CLIENT_HELLO. A periodic sweep (every 300 ms, on a task thread) resumes queued connections when slots free up and expires ones that have waited longer than `max_age`. **Slot-counter underflow.** A queued connection never increments the active-slot counter, but closing one decremented it unconditionally. The counter wraps below zero, and the next reservation fails a release assert, aborting the server. The sweep had a related accounting bug: its loop condition was inverted, so it resumed a queued connection precisely when `reserve()` failed — waking it without a slot — and stopped, discarding a successful reservation, when `reserve()` succeeded. The first commit balances the accounting: - the sweep reserves a slot first and only then dequeues and resumes a connection, so every resumed connection owns the slot its close releases; - closing a connection releases a slot only if the connection is no longer queued, and drops a still-queued connection from the queue — which also removes a stale queue entry (dangling VC pointer) left behind when a parked connection is reset; - a connection expired by `max_age` is detached (user arg cleared, selector lease released) the same way the reject path does, so its close no longer releases a slot it never held. Queued connections now stay parked until a slot actually frees instead of being resumed over the limit — which also makes `max_age` expiry reachable. **Sweep/close synchronization.** The sweep and the VCONN_CLOSE handler (net thread, inline) update the queue, the slot counter, and the selector lease in multi-step sequences. Each structure is locked on its own, but the sequences can interleave: a close can empty the queue between the sweep's `size()` and `pop()` (the sweep then reenables a null VC), release a slot the sweep is concurrently granting, double-release the selector lease and delete the selector early, or free a popped connection before the sweep reenables it. The second commit synchronizes the sweep with connection close under one plugin mutex: the sweep's reserve→pop→reenable sequence and the close handler's slot/lease accounting run under the lock, the close handler re-reads the VC user arg under the lock (the expiry path may have detached the VC), and `pop()` results are null-checked. **Add autests for this plugin.** Three tests in `tests/gold_tests/pluginTest/rate_limit/`, the first autest directory for rate_limit (bash + `openssl s_client` clients against a TLS-enabled ATS with `limit: 1`): - `rate_limit_sni_queue` — queue size 1; one connection holds the single slot (a FIFO keeps it open until the test releases it), a second parks in the queue and closes while parked; - `rate_limit_sni_expiry` — same, with the parked connection expired by `max_age: 1`; - `rate_limit_sni_reject` — no queue; a burst of connections over the limit is rejected mid-handshake (`TSVConnReenableEx` with `TS_EVENT_ERROR`). With an unpatched plugin, the queue and expiry tests fail: traffic_server exits on SIGABRT at `failed assertion '_active <= _limit'` (limiter.h), and the underflow is visible in traffic.out (`Releasing a slot, active entities == 0` followed by `... == 4294967295`). With these commits they pass and the logs contain no assertion, Fatal, or signal lines. The abort under test is a `TSReleaseAssert`, so the tests do not depend on ASan or a particular build type. The reject test passes with and without the fixes — it is coverage of the no-queue reject path, which the fixes do not touch. The accounting fix is regression-tested on both the resume and `max_age` paths. The race fix is analytic: the interleavings above follow from the threading model, but a lost race has no deterministic reproduction, so there is no regression test for the second commit. -- 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]
