From 094e66feb31d5680fd04b3bf28ed3e24ebc96766 Mon Sep 17 00:00:00 2001
From: Mo Chen <mochen@apache.org>
Date: Sun, 12 Jul 2026 17:47:45 -0500
Subject: [PATCH 1/5] rate_limit: balance the SNI active-slot counter for
 queued connections

A queued SNI connection never reserves a slot, but its VCONN_CLOSE
released one unconditionally. A queued connection that closed therefore
decremented the active-slot counter without a matching increment; it
wrapped below zero and the next reserve() aborted the server on
TSReleaseAssert(_active <= _limit).

Balance the accounting: resume queued connections with reserve-then-pop
so a resumed connection owns a real slot; release a slot on close only
when the connection is no longer queued (a still-queued one never held
one) and drop it from the queue; detach an expired connection the same
way the reject path does. Removing a closing connection from the queue
also fixes a stale-pointer dereference when a parked queued connection
is reset.

Add deterministic regressions for the resume and max_age paths.
---
 plugins/experimental/rate_limit/limiter.h     | 19 ++++
 .../experimental/rate_limit/sni_limiter.cc    |  7 +-
 .../experimental/rate_limit/sni_selector.cc   | 21 ++++-
 .../rate_limit/rate_limit_sni_expiry.test.py  | 87 ++++++++++++++++++
 .../rate_limit_sni_expiry_client.sh           | 63 +++++++++++++
 .../rate_limit/rate_limit_sni_queue.test.py   | 89 +++++++++++++++++++
 .../rate_limit/rate_limit_sni_queue_client.sh | 64 +++++++++++++
 7 files changed, 346 insertions(+), 4 deletions(-)
 create mode 100644 tests/gold_tests/pluginTest/rate_limit/rate_limit_sni_expiry.test.py
 create mode 100644 tests/gold_tests/pluginTest/rate_limit/rate_limit_sni_expiry_client.sh
 create mode 100644 tests/gold_tests/pluginTest/rate_limit/rate_limit_sni_queue.test.py
 create mode 100644 tests/gold_tests/pluginTest/rate_limit/rate_limit_sni_queue_client.sh

diff --git a/plugins/experimental/rate_limit/limiter.h b/plugins/experimental/rate_limit/limiter.h
index 6696274fafc..74979ad1d91 100644
--- a/plugins/experimental/rate_limit/limiter.h
+++ b/plugins/experimental/rate_limit/limiter.h
@@ -327,6 +327,25 @@ template <class T> class RateLimiter
     return item;
   }
 
+  // Remove a still-queued element (e.g. a connection that closed before it was resumed).
+  // Returns true if it was found in the queue, so the caller can tell a queued element
+  // (which never reserved a slot) from one that was already resumed.
+  bool
+  remove(T elem)
+  {
+    std::lock_guard<std::mutex> lock(_queue_lock);
+
+    for (auto it = _queue.begin(); it != _queue.end(); ++it) {
+      if (std::get<0>(*it) == elem) {
+        _queue.erase(it);
+        --_size;
+        return true;
+      }
+    }
+
+    return false;
+  }
+
   void
   incrementMetric(uint metric)
   {
diff --git a/plugins/experimental/rate_limit/sni_limiter.cc b/plugins/experimental/rate_limit/sni_limiter.cc
index 67241ad1784..36bca010045 100644
--- a/plugins/experimental/rate_limit/sni_limiter.cc
+++ b/plugins/experimental/rate_limit/sni_limiter.cc
@@ -171,7 +171,12 @@ sni_limit_cont(TSCont contp, TSEvent event, void *edata)
 
     if (limiter) {
       TSUserArgSet(vc, gVCIdx, nullptr);
-      limiter->free();
+      // A connection that is still queued never reserved a slot, so only release one if it
+      // is not in the queue (either it reserved at CLIENT_HELLO or the sweep resumed it into
+      // a reserved slot). Dropping it from the queue also avoids a stale entry.
+      if (!limiter->remove(vc)) {
+        limiter->free();
+      }
       limiter->selector()->release(); // Release the selector, such that it can be deleted later
     }
     TSVConnReenable(vc);
diff --git a/plugins/experimental/rate_limit/sni_selector.cc b/plugins/experimental/rate_limit/sni_selector.cc
index c1c2eec7ea7..5d992687aa0 100644
--- a/plugins/experimental/rate_limit/sni_selector.cc
+++ b/plugins/experimental/rate_limit/sni_selector.cc
@@ -219,9 +219,17 @@ sni_queue_cont(TSCont cont, TSEvent /* event ATS_UNUSED */, void * /* edata ATS_
     QueueTime now     = std::chrono::system_clock::now(); // Only do this once per limiter
 
     if (owner) { // Don't operate on the aliases
-      // Try to enable some queued VCs (if any) if there are slots available
+      // Try to enable some queued VCs (if any) if there are slots available. Reserving before
+      // dequeuing means a resumed VC owns the slot it was granted, so its VCONN_CLOSE releases
+      // exactly that slot.
       while (limiter->size() > 0 && limiter->reserve() == ReserveStatus::RESERVED) {
-        auto [vc, contp, start_time]    = limiter->pop();
+        auto [vc, contp, start_time] = limiter->pop();
+
+        if (nullptr == vc) { // A concurrent close emptied the queue; give the slot back
+          limiter->free();
+          break;
+        }
+
         std::chrono::milliseconds delay = std::chrono::duration_cast<std::chrono::milliseconds>(now - start_time);
 
         (void)contp; // Ugly, but silences some compilers.
@@ -236,11 +244,18 @@ sni_queue_cont(TSCont cont, TSEvent /* event ATS_UNUSED */, void * /* edata ATS_
 
         while (limiter->size() > 0 && limiter->hasOldEntity(now)) {
           // The oldest object on the queue is too old on the queue, so "kill" it.
-          auto [vc, contp, start_time]  = limiter->pop();
+          auto [vc, contp, start_time] = limiter->pop();
+
+          if (nullptr == vc) { // A concurrent close emptied the queue
+            break;
+          }
+
           std::chrono::milliseconds age = std::chrono::duration_cast<std::chrono::milliseconds>(now - start_time);
 
           (void)contp;
           Dbg(dbg_ctl, "Queued VC is too old (%ldms), erroring out", static_cast<long>(age.count()));
+          // This VC never reserved a slot; detach it (clear the arg and release the selector
+          // lease) so its VCONN_CLOSE does not release a slot it never held.
           TSUserArgSet(vc, gVCIdx, nullptr);
           limiter->selector()->release();
           TSVConnReenableEx(vc, TS_EVENT_ERROR);
diff --git a/tests/gold_tests/pluginTest/rate_limit/rate_limit_sni_expiry.test.py b/tests/gold_tests/pluginTest/rate_limit/rate_limit_sni_expiry.test.py
new file mode 100644
index 00000000000..d0a915223a6
--- /dev/null
+++ b/tests/gold_tests/pluginTest/rate_limit/rate_limit_sni_expiry.test.py
@@ -0,0 +1,87 @@
+'''
+Regression test for the max_age expiry branch of the rate_limit SNI queue accounting. A
+queued connection never reserves a slot, so when the sweep expires it the plugin must
+detach it rather than release a slot it never held; otherwise the expiry underflows the
+active-slot counter and the next reserve() trips a release assertion, aborting the server.
+ATS must survive the expiry.
+'''
+#  Licensed to the Apache Software Foundation (ASF) under one
+#  or more contributor license agreements.  See the NOTICE file
+#  distributed with this work for additional information
+#  regarding copyright ownership.  The ASF licenses this file
+#  to you under the Apache License, Version 2.0 (the
+#  "License"); you may not use this file except in compliance
+#  with the License.  You may obtain a copy of the License at
+#
+#      http://www.apache.org/licenses/LICENSE-2.0
+#
+#  Unless required by applicable law or agreed to in writing, software
+#  distributed under the License is distributed on an "AS IS" BASIS,
+#  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+#  See the License for the specific language governing permissions and
+#  limitations under the License.
+
+import os
+
+Test.Summary = __doc__
+
+Test.SkipUnless(Condition.PluginExists('rate_limit.so'))
+
+
+class RateLimitSniExpiryTest:
+    """Age a queued connection out via max_age and assert the active-slot counter stays balanced."""
+
+    def __init__(self) -> None:
+        tr = Test.AddTestRun('rate_limit SNI queue max_age expiry')
+        self._configure_trafficserver()
+        self._configure_client(tr)
+
+    def _configure_trafficserver(self) -> None:
+        ts = Test.MakeATSProcess('ts', enable_tls=True, enable_cache=False)
+        self._ts = ts
+        ts.addDefaultSSLFiles()
+        for line in ['ssl_multicert:', '  - dest_ip: "*"', '    ssl_cert_name: server.pem', '    ssl_key_name: server.key']:
+            ts.Disk.ssl_multicert_yaml.AddLine(line)
+
+        # One concurrent handshake for this SNI, a one-deep queue, and a 1s max age so the
+        # sweep expires the queued connection. Named .config (not .yaml) so autest treats it
+        # as a plain config file; the plugin parses it as YAML regardless.
+        ts.Disk.MakeConfigFile('rate_limit.config').AddLines(
+            [
+                'selector:',
+                '  - sni: rate.limited.com',
+                '    limit: 1',
+                '    queue:',
+                '      size: 1',
+                '      max_age: 1',
+            ])
+        ts.Disk.plugin_config.AddLine(f'rate_limit.so {ts.Variables.CONFIGDIR}/rate_limit.config')
+
+        # Disable the freelist / ProxyAllocator so allocation behavior is not a confound; the
+        # abort under test is a release-assertion, and this keeps the run representative of CI.
+        ts.Command += ' -f -F'
+
+        ts.Disk.records_config.update(
+            {
+                'proxy.config.ssl.server.cert.path': ts.Variables.SSLDir,
+                'proxy.config.ssl.server.private_key.path': ts.Variables.SSLDir,
+                'proxy.config.diags.debug.enabled': 1,
+                'proxy.config.diags.debug.tags': 'rate_limit',
+            })
+
+        # The expiry branch is actually reached...
+        ts.Disk.traffic_out.Content = Testers.ContainsExpression('too old', 'a queued connection was expired')
+        # ...and expiring it does not underflow the active-slot counter into the release assertion.
+        ts.Disk.traffic_out.Content += Testers.ExcludesExpression(
+            '_active <= _limit|received signal', 'expiring a queued connection must not underflow and abort ATS')
+
+    def _configure_client(self, tr) -> None:
+        ts = self._ts
+        client = os.path.join(Test.TestDirectory, 'rate_limit_sni_expiry_client.sh')
+        tr.Processes.Default.Command = f'bash {client} 127.0.0.1 {ts.Variables.ssl_port} rate.limited.com'
+        tr.Processes.Default.ReturnCode = 0
+        tr.Processes.Default.StartBefore(ts)
+        tr.Processes.Default.Streams.stdout = Testers.ContainsExpression('rate_limit-expiry-done', 'the client ran to completion')
+
+
+RateLimitSniExpiryTest()
diff --git a/tests/gold_tests/pluginTest/rate_limit/rate_limit_sni_expiry_client.sh b/tests/gold_tests/pluginTest/rate_limit/rate_limit_sni_expiry_client.sh
new file mode 100644
index 00000000000..eb57153b3d5
--- /dev/null
+++ b/tests/gold_tests/pluginTest/rate_limit/rate_limit_sni_expiry_client.sh
@@ -0,0 +1,63 @@
+#!/usr/bin/env bash
+#
+#  Licensed to the Apache Software Foundation (ASF) under one
+#  or more contributor license agreements.  See the NOTICE file
+#  distributed with this work for additional information
+#  regarding copyright ownership.  The ASF licenses this file
+#  to you under the Apache License, Version 2.0 (the
+#  "License"); you may not use this file except in compliance
+#  with the License.  You may obtain a copy of the License at
+#
+#      http://www.apache.org/licenses/LICENSE-2.0
+#
+#  Unless required by applicable law or agreed to in writing, software
+#  distributed under the License is distributed on an "AS IS" BASIS,
+#  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+#  See the License for the specific language governing permissions and
+#  limitations under the License.
+#
+# Exercise the max_age EXPIRY branch of the rate_limit SNI queue accounting. A queued
+# connection never reserves a slot; when the sweep expires it (max_age), it must be
+# detached so its close does not release a slot it never held. Otherwise the expiry is an
+# unmatched decrement of the active-slot counter, and -- combined with the holder's own
+# close -- the counter wraps below zero and the limiter's release assertion aborts ATS.
+#
+#   1. holder completes its handshake and holds the single slot (counter = 1);
+#   2. one connection enqueues (slot full) and stays parked -- it is NOT disconnected, so
+#      only the sweep's max_age expiry removes it;
+#   3. after max_age the sweep errors it out -> (unfixed) unmatched decrement -> counter 1->0;
+#   4. the holder is closed; its matched decrement lands on the understated counter -> wrap;
+#   5. a probe connection's reserve() observes the wrapped counter and the assertion aborts.
+#
+# args: host port sni
+set -u
+host="$1"
+port="$2"
+sni="$3"
+
+OSSL="openssl s_client -connect ${host}:${port} -servername ${sni} -quiet -verify_quiet -no_ign_eof"
+
+# 1. Holder: hold the single slot. Its stdin is a FIFO on fd 3 so we end it in step 4.
+fifo="$(mktemp -u "${TMPDIR:-/tmp}/rl_holder.XXXXXX")"
+mkfifo "$fifo"
+${OSSL} <"$fifo" >/dev/null 2>&1 &
+exec 3<>"$fifo"
+rm -f "$fifo"
+sleep 3 # let the holder reserve the one slot
+
+# 2. One queued connection: enqueues and stays parked at the ClientHello hook (not killed),
+#    so the sweep's max_age expiry -- not a disconnect or a resume -- is what removes it.
+${OSSL} </dev/null >/dev/null 2>&1 &
+queued=$!
+sleep 3 # > max_age (1s) + sweeps: the expiry path errors the queued connection out
+
+# 4. End the holder: its matched decrement lands on the (unfixed) understated counter.
+exec 3>&-
+sleep 2
+
+# 5. Probe: its reserve() reads the counter; if it wrapped, the release assertion aborts.
+timeout 2 ${OSSL} </dev/null >/dev/null 2>&1 || true
+kill "${queued}" 2>/dev/null || true
+sleep 1
+
+echo "rate_limit-expiry-done"
diff --git a/tests/gold_tests/pluginTest/rate_limit/rate_limit_sni_queue.test.py b/tests/gold_tests/pluginTest/rate_limit/rate_limit_sni_queue.test.py
new file mode 100644
index 00000000000..cad24fbd777
--- /dev/null
+++ b/tests/gold_tests/pluginTest/rate_limit/rate_limit_sni_queue.test.py
@@ -0,0 +1,89 @@
+'''
+Regression test for a queue-accounting balance bug in the rate_limit SNI limiter: a
+queued connection never reserves a slot, but its VCONN_CLOSE unconditionally releases
+one, so a queued connection that closes underflows the active-slot counter and the next
+reserve() trips a release assertion, aborting the server. ATS must survive the queue
+churn.
+'''
+#  Licensed to the Apache Software Foundation (ASF) under one
+#  or more contributor license agreements.  See the NOTICE file
+#  distributed with this work for additional information
+#  regarding copyright ownership.  The ASF licenses this file
+#  to you under the Apache License, Version 2.0 (the
+#  "License"); you may not use this file except in compliance
+#  with the License.  You may obtain a copy of the License at
+#
+#      http://www.apache.org/licenses/LICENSE-2.0
+#
+#  Unless required by applicable law or agreed to in writing, software
+#  distributed under the License is distributed on an "AS IS" BASIS,
+#  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+#  See the License for the specific language governing permissions and
+#  limitations under the License.
+
+import os
+
+Test.Summary = __doc__
+
+Test.SkipUnless(Condition.PluginExists('rate_limit.so'))
+
+
+class RateLimitSniQueueTest:
+    """Churn the rate_limit SNI queue and assert the active-slot counter never underflows."""
+
+    def __init__(self) -> None:
+        tr = Test.AddTestRun('rate_limit SNI queue accounting')
+        self._configure_trafficserver()
+        self._configure_client(tr)
+
+    def _configure_trafficserver(self) -> None:
+        ts = Test.MakeATSProcess('ts', enable_tls=True, enable_cache=False)
+        self._ts = ts
+        ts.addDefaultSSLFiles()
+        for line in ['ssl_multicert:', '  - dest_ip: "*"', '    ssl_cert_name: server.pem', '    ssl_key_name: server.key']:
+            ts.Disk.ssl_multicert_yaml.AddLine(line)
+
+        # One concurrent handshake for this SNI and a queue that admits exactly one more.
+        # No rate and no max_age -- the sweep's resume path alone drives the scenario, with
+        # no rate-bucket or expiry timing to confound it. Named .config (not .yaml) so autest
+        # treats it as a plain config file; the plugin parses it as YAML regardless.
+        ts.Disk.MakeConfigFile('rate_limit.config').AddLines(
+            [
+                'selector:',
+                '  - sni: rate.limited.com',
+                '    limit: 1',
+                '    queue:',
+                '      size: 1',
+            ])
+        ts.Disk.plugin_config.AddLine(f'rate_limit.so {ts.Variables.CONFIGDIR}/rate_limit.config')
+
+        # Disable the freelist / ProxyAllocator so freed objects are really released rather
+        # than recycled, keeping allocation reuse from masking a stale access.
+        ts.Command += ' -f -F'
+
+        ts.Disk.records_config.update(
+            {
+                'proxy.config.ssl.server.cert.path': ts.Variables.SSLDir,
+                'proxy.config.ssl.server.private_key.path': ts.Variables.SSLDir,
+                'proxy.config.diags.debug.enabled': 1,
+                'proxy.config.diags.debug.tags': 'rate_limit',
+            })
+
+        # The queue path is reached...
+        ts.Disk.traffic_out.Content = Testers.ContainsExpression('Queueing the VC', 'a connection was queued')
+        # ...and the active-slot counter never underflows into the release assertion. Match
+        # both the specific assertion (pins the failure to this bug) and the generic abort.
+        ts.Disk.traffic_out.Content += Testers.ExcludesExpression(
+            '_active <= _limit|received signal', 'the active-slot counter must not underflow and abort ATS')
+
+    def _configure_client(self, tr) -> None:
+        ts = self._ts
+        client = os.path.join(Test.TestDirectory, 'rate_limit_sni_queue_client.sh')
+        tr.Processes.Default.Command = f'bash {client} 127.0.0.1 {ts.Variables.ssl_port} rate.limited.com'
+        tr.Processes.Default.ReturnCode = 0
+        tr.Processes.Default.StartBefore(ts)
+        tr.Processes.Default.Streams.stdout = Testers.ContainsExpression(
+            'rate_limit-queue-crash-done', 'the client ran to completion')
+
+
+RateLimitSniQueueTest()
diff --git a/tests/gold_tests/pluginTest/rate_limit/rate_limit_sni_queue_client.sh b/tests/gold_tests/pluginTest/rate_limit/rate_limit_sni_queue_client.sh
new file mode 100644
index 00000000000..87af9fb62f0
--- /dev/null
+++ b/tests/gold_tests/pluginTest/rate_limit/rate_limit_sni_queue_client.sh
@@ -0,0 +1,64 @@
+#!/usr/bin/env bash
+#
+#  Licensed to the Apache Software Foundation (ASF) under one
+#  or more contributor license agreements.  See the NOTICE file
+#  distributed with this work for additional information
+#  regarding copyright ownership.  The ASF licenses this file
+#  to you under the Apache License, Version 2.0 (the
+#  "License"); you may not use this file except in compliance
+#  with the License.  You may obtain a copy of the License at
+#
+#      http://www.apache.org/licenses/LICENSE-2.0
+#
+#  Unless required by applicable law or agreed to in writing, software
+#  distributed under the License is distributed on an "AS IS" BASIS,
+#  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+#  See the License for the specific language governing permissions and
+#  limitations under the License.
+#
+# Deterministically drive the rate_limit SNI limiter's queue-accounting balance bug. A
+# queued connection never increments the active-slot counter, but its VCONN_CLOSE always
+# decrements it, so one queued connection that closes is a single unmatched decrement.
+# With exactly one queued connection there is no way for the sweep to mask it:
+#
+#   1. holder completes its handshake and holds the single slot (counter = 1);
+#   2. one connection enqueues (slot full), then closes cleanly (FIN) while parked;
+#   3. the sweep resumes it, its handshake fails and it closes -> one unmatched decrement
+#      -> counter 1 -> 0 (the queue is now empty, so no reserve() can rebalance it);
+#   4. the holder is closed; its matched decrement lands on the understated counter
+#      -> counter 0 -> wraps below zero;
+#   5. a probe connection's reserve() observes the wrapped counter and the limiter's
+#      release assertion (_active <= _limit) aborts the server.
+#
+# args: host port sni
+set -u
+host="$1"
+port="$2"
+sni="$3"
+
+OSSL="openssl s_client -connect ${host}:${port} -servername ${sni} -quiet -verify_quiet -no_ign_eof"
+
+# 1. Holder: hold the single slot. Its stdin is a FIFO kept open on fd 3, so we end the
+#    holder deterministically in step 4 (closing fd 3 -> EOF -> clean TLS close -> FIN).
+fifo="$(mktemp -u "${TMPDIR:-/tmp}/rl_holder.XXXXXX")"
+mkfifo "$fifo"
+${OSSL} <"$fifo" >/dev/null 2>&1 &
+exec 3<>"$fifo"
+rm -f "$fifo"
+sleep 3 # let the holder reserve the one slot
+
+# 2. One queued connection: enqueues (slot full), then sends a clean FIN ~0.3s later while
+#    still parked at the ClientHello hook.
+timeout 0.3 ${OSSL} </dev/null >/dev/null 2>&1 &
+sleep 2 # >= 2 sweep periods (300ms each): the sweep resumes the queued connection, its
+        # handshake fails (EPIPE) and it closes -> one unmatched decrement -> counter 1 -> 0
+
+# 4. End the holder: its matched decrement lands on the already-understated counter.
+exec 3>&- # close the FIFO write end -> holder sees EOF -> clean TLS close (FIN)
+sleep 2   # let the holder's close run: counter 0 -> wraps below zero
+
+# 5. Probe: its reserve() reads the wrapped counter and trips TSReleaseAssert(_active <= _limit).
+timeout 2 ${OSSL} </dev/null >/dev/null 2>&1 || true
+sleep 1
+
+echo "rate_limit-queue-crash-done"

From 46ace452d4b467760ce24b23a9054027d4b0702a Mon Sep 17 00:00:00 2001
From: Mo Chen <mochen@apache.org>
Date: Sun, 12 Jul 2026 17:48:47 -0500
Subject: [PATCH 2/5] rate_limit: add an SNI reject-teardown autest

Exercise the sync-reject path against a TLS listener: a holder reserves
the one slot and a burst of concurrent handshakes is rejected
mid-handshake (TS_EVENT_ERROR) with the allocator freelists disabled.
Asserts the reject path is reached and every rejected handshake VC is
freed without a memory-safety fault.
---
 .../rate_limit/rate_limit_sni_reject.test.py  | 83 +++++++++++++++++++
 .../rate_limit_sni_reject_client.sh           | 47 +++++++++++
 2 files changed, 130 insertions(+)
 create mode 100644 tests/gold_tests/pluginTest/rate_limit/rate_limit_sni_reject.test.py
 create mode 100644 tests/gold_tests/pluginTest/rate_limit/rate_limit_sni_reject_client.sh

diff --git a/tests/gold_tests/pluginTest/rate_limit/rate_limit_sni_reject.test.py b/tests/gold_tests/pluginTest/rate_limit/rate_limit_sni_reject.test.py
new file mode 100644
index 00000000000..7d32c8dda15
--- /dev/null
+++ b/tests/gold_tests/pluginTest/rate_limit/rate_limit_sni_reject.test.py
@@ -0,0 +1,83 @@
+'''
+Exercise the rate_limit SNI limiter's reject path against a TLS listener, so the
+consumer-driven SSLNetVConnection teardown frees every rejected handshake VC
+cleanly (no use-after-free or crash).
+'''
+#  Licensed to the Apache Software Foundation (ASF) under one
+#  or more contributor license agreements.  See the NOTICE file
+#  distributed with this work for additional information
+#  regarding copyright ownership.  The ASF licenses this file
+#  to you under the Apache License, Version 2.0 (the
+#  "License"); you may not use this file except in compliance
+#  with the License.  You may obtain a copy of the License at
+#
+#      http://www.apache.org/licenses/LICENSE-2.0
+#
+#  Unless required by applicable law or agreed to in writing, software
+#  distributed under the License is distributed on an "AS IS" BASIS,
+#  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+#  See the License for the specific language governing permissions and
+#  limitations under the License.
+
+import os
+
+Test.Summary = __doc__
+
+Test.SkipUnless(Condition.PluginExists('rate_limit.so'))
+
+
+class RateLimitSniRejectTest:
+    """Drive rate_limit's SNI reject path and assert ATS frees the VCs without a fault."""
+
+    def __init__(self) -> None:
+        tr = Test.AddTestRun('rate_limit SNI reject teardown')
+        self._configure_trafficserver()
+        self._configure_client(tr)
+
+    def _configure_trafficserver(self) -> None:
+        ts = Test.MakeATSProcess('ts', enable_tls=True, enable_cache=False)
+        self._ts = ts
+        ts.addDefaultSSLFiles()
+        for line in ['ssl_multicert:', '  - dest_ip: "*"', '    ssl_cert_name: server.pem', '    ssl_key_name: server.key']:
+            ts.Disk.ssl_multicert_yaml.AddLine(line)
+
+        # One concurrent handshake for this SNI and no queue, so every further concurrent
+        # handshake is rejected outright (TS_EVENT_ERROR) rather than queued. Named .config
+        # (not .yaml) so autest treats it as a plain config file; the plugin parses it as
+        # YAML regardless (YAML::LoadFile).
+        ts.Disk.MakeConfigFile('rate_limit.config').AddLines([
+            'selector:',
+            '  - sni: rate.limited.com',
+            '    limit: 1',
+        ])
+        ts.Disk.plugin_config.AddLine(f'rate_limit.so {ts.Variables.CONFIGDIR}/rate_limit.config')
+
+        # Disable the freelist / ProxyAllocator so a freed SSLNetVConnection is really
+        # free()'d rather than recycled; a stale-VC access then hits freed memory
+        # instead of a still-valid recycled object.
+        ts.Command += ' -f -F'
+
+        ts.Disk.records_config.update(
+            {
+                'proxy.config.ssl.server.cert.path': ts.Variables.SSLDir,
+                'proxy.config.ssl.server.private_key.path': ts.Variables.SSLDir,
+                'proxy.config.diags.debug.enabled': 1,
+                'proxy.config.diags.debug.tags': 'rate_limit',
+            })
+
+        # The reject disposition is reached...
+        ts.Disk.traffic_out.Content = Testers.ContainsExpression('Rejecting connection', 'over-limit handshakes were rejected')
+        # ...and ATS tears every rejected handshake VC down without a memory-safety fault.
+        ts.Disk.traffic_out.Content += Testers.ExcludesExpression(
+            'use-after-free|attempting free|SEGV|received signal', 'ATS must survive the reject churn')
+
+    def _configure_client(self, tr) -> None:
+        ts = self._ts
+        client = os.path.join(Test.TestDirectory, 'rate_limit_sni_reject_client.sh')
+        tr.Processes.Default.Command = f'bash {client} 127.0.0.1 {ts.Variables.ssl_port} rate.limited.com'
+        tr.Processes.Default.ReturnCode = 0
+        tr.Processes.Default.StartBefore(ts)
+        tr.Processes.Default.Streams.stdout = Testers.ContainsExpression('rate_limit-reject-done', 'the client ran to completion')
+
+
+RateLimitSniRejectTest()
diff --git a/tests/gold_tests/pluginTest/rate_limit/rate_limit_sni_reject_client.sh b/tests/gold_tests/pluginTest/rate_limit/rate_limit_sni_reject_client.sh
new file mode 100644
index 00000000000..bd1f8e07b6f
--- /dev/null
+++ b/tests/gold_tests/pluginTest/rate_limit/rate_limit_sni_reject_client.sh
@@ -0,0 +1,47 @@
+#!/usr/bin/env bash
+#
+#  Licensed to the Apache Software Foundation (ASF) under one
+#  or more contributor license agreements.  See the NOTICE file
+#  distributed with this work for additional information
+#  regarding copyright ownership.  The ASF licenses this file
+#  to you under the Apache License, Version 2.0 (the
+#  "License"); you may not use this file except in compliance
+#  with the License.  You may obtain a copy of the License at
+#
+#      http://www.apache.org/licenses/LICENSE-2.0
+#
+#  Unless required by applicable law or agreed to in writing, software
+#  distributed under the License is distributed on an "AS IS" BASIS,
+#  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+#  See the License for the specific language governing permissions and
+#  limitations under the License.
+#
+# Drive the rate_limit SNI limiter (limit 1, no queue) through its reject path so the
+# consumer-driven SSLNetVConnection teardown is exercised for a rejected handshake:
+#   holder completes the handshake and HOLDS the one slot open;
+#   a burst of near-simultaneous handshakes then arrives while the slot is taken and,
+#         with no queue configured, each is REJECTED with TS_EVENT_ERROR mid-handshake.
+# ATS must free every one of these rejected handshake VCs cleanly.
+#
+# args: host port sni
+set -u
+host="$1"
+port="$2"
+sni="$3"
+
+OSSL="openssl s_client -connect ${host}:${port} -servername ${sni} -quiet -verify_quiet -no_ign_eof"
+
+# holder: complete the handshake and hold the single slot for ~5s (slow stdin keeps it open).
+(sleep 5) | ${OSSL} >/dev/null 2>&1 &
+sleep 2 # let the holder reserve the slot
+
+# Burst of near-simultaneous handshakes against the full limiter; with no queue every one
+# is rejected with TS_EVENT_ERROR, so its handshake VC is torn down consumer-driven.
+for _ in $(seq 5); do
+  timeout 2 ${OSSL} </dev/null >/dev/null 2>&1 &
+done
+
+# Let the burst finish and the holder release its slot cleanly.
+sleep 4
+
+echo "rate_limit-reject-done"

From e724b5429f811363a8c7645157c0c5744b10807e Mon Sep 17 00:00:00 2001
From: Mo Chen <mochen@apache.org>
Date: Sun, 19 Jul 2026 13:21:16 -0500
Subject: [PATCH 3/5] rate_limit tests: annotate helpers and create the FIFO
 atomically

Annotate the TestRun parameters like the surrounding class-based gold
tests, and create the holder FIFO inside a fresh mktemp -d directory
instead of on an unlinked mktemp -u path, whose creation is not atomic.
---
 .../pluginTest/rate_limit/rate_limit_sni_expiry.test.py      | 2 +-
 .../pluginTest/rate_limit/rate_limit_sni_expiry_client.sh    | 5 +++--
 .../pluginTest/rate_limit/rate_limit_sni_queue.test.py       | 2 +-
 .../pluginTest/rate_limit/rate_limit_sni_queue_client.sh     | 5 +++--
 .../pluginTest/rate_limit/rate_limit_sni_reject.test.py      | 2 +-
 5 files changed, 9 insertions(+), 7 deletions(-)

diff --git a/tests/gold_tests/pluginTest/rate_limit/rate_limit_sni_expiry.test.py b/tests/gold_tests/pluginTest/rate_limit/rate_limit_sni_expiry.test.py
index d0a915223a6..8ae995bf1c4 100644
--- a/tests/gold_tests/pluginTest/rate_limit/rate_limit_sni_expiry.test.py
+++ b/tests/gold_tests/pluginTest/rate_limit/rate_limit_sni_expiry.test.py
@@ -75,7 +75,7 @@ def _configure_trafficserver(self) -> None:
         ts.Disk.traffic_out.Content += Testers.ExcludesExpression(
             '_active <= _limit|received signal', 'expiring a queued connection must not underflow and abort ATS')
 
-    def _configure_client(self, tr) -> None:
+    def _configure_client(self, tr: 'TestRun') -> None:
         ts = self._ts
         client = os.path.join(Test.TestDirectory, 'rate_limit_sni_expiry_client.sh')
         tr.Processes.Default.Command = f'bash {client} 127.0.0.1 {ts.Variables.ssl_port} rate.limited.com'
diff --git a/tests/gold_tests/pluginTest/rate_limit/rate_limit_sni_expiry_client.sh b/tests/gold_tests/pluginTest/rate_limit/rate_limit_sni_expiry_client.sh
index eb57153b3d5..73fe04b62c8 100644
--- a/tests/gold_tests/pluginTest/rate_limit/rate_limit_sni_expiry_client.sh
+++ b/tests/gold_tests/pluginTest/rate_limit/rate_limit_sni_expiry_client.sh
@@ -38,11 +38,12 @@ sni="$3"
 OSSL="openssl s_client -connect ${host}:${port} -servername ${sni} -quiet -verify_quiet -no_ign_eof"
 
 # 1. Holder: hold the single slot. Its stdin is a FIFO on fd 3 so we end it in step 4.
-fifo="$(mktemp -u "${TMPDIR:-/tmp}/rl_holder.XXXXXX")"
+fifo_dir="$(mktemp -d "${TMPDIR:-/tmp}/rl_holder.XXXXXX")"
+fifo="${fifo_dir}/fifo"
 mkfifo "$fifo"
 ${OSSL} <"$fifo" >/dev/null 2>&1 &
 exec 3<>"$fifo"
-rm -f "$fifo"
+rm -rf "$fifo_dir"
 sleep 3 # let the holder reserve the one slot
 
 # 2. One queued connection: enqueues and stays parked at the ClientHello hook (not killed),
diff --git a/tests/gold_tests/pluginTest/rate_limit/rate_limit_sni_queue.test.py b/tests/gold_tests/pluginTest/rate_limit/rate_limit_sni_queue.test.py
index cad24fbd777..59042752620 100644
--- a/tests/gold_tests/pluginTest/rate_limit/rate_limit_sni_queue.test.py
+++ b/tests/gold_tests/pluginTest/rate_limit/rate_limit_sni_queue.test.py
@@ -76,7 +76,7 @@ def _configure_trafficserver(self) -> None:
         ts.Disk.traffic_out.Content += Testers.ExcludesExpression(
             '_active <= _limit|received signal', 'the active-slot counter must not underflow and abort ATS')
 
-    def _configure_client(self, tr) -> None:
+    def _configure_client(self, tr: 'TestRun') -> None:
         ts = self._ts
         client = os.path.join(Test.TestDirectory, 'rate_limit_sni_queue_client.sh')
         tr.Processes.Default.Command = f'bash {client} 127.0.0.1 {ts.Variables.ssl_port} rate.limited.com'
diff --git a/tests/gold_tests/pluginTest/rate_limit/rate_limit_sni_queue_client.sh b/tests/gold_tests/pluginTest/rate_limit/rate_limit_sni_queue_client.sh
index 87af9fb62f0..f8269cf8b7b 100644
--- a/tests/gold_tests/pluginTest/rate_limit/rate_limit_sni_queue_client.sh
+++ b/tests/gold_tests/pluginTest/rate_limit/rate_limit_sni_queue_client.sh
@@ -40,11 +40,12 @@ OSSL="openssl s_client -connect ${host}:${port} -servername ${sni} -quiet -verif
 
 # 1. Holder: hold the single slot. Its stdin is a FIFO kept open on fd 3, so we end the
 #    holder deterministically in step 4 (closing fd 3 -> EOF -> clean TLS close -> FIN).
-fifo="$(mktemp -u "${TMPDIR:-/tmp}/rl_holder.XXXXXX")"
+fifo_dir="$(mktemp -d "${TMPDIR:-/tmp}/rl_holder.XXXXXX")"
+fifo="${fifo_dir}/fifo"
 mkfifo "$fifo"
 ${OSSL} <"$fifo" >/dev/null 2>&1 &
 exec 3<>"$fifo"
-rm -f "$fifo"
+rm -rf "$fifo_dir"
 sleep 3 # let the holder reserve the one slot
 
 # 2. One queued connection: enqueues (slot full), then sends a clean FIN ~0.3s later while
diff --git a/tests/gold_tests/pluginTest/rate_limit/rate_limit_sni_reject.test.py b/tests/gold_tests/pluginTest/rate_limit/rate_limit_sni_reject.test.py
index 7d32c8dda15..976d320deb1 100644
--- a/tests/gold_tests/pluginTest/rate_limit/rate_limit_sni_reject.test.py
+++ b/tests/gold_tests/pluginTest/rate_limit/rate_limit_sni_reject.test.py
@@ -71,7 +71,7 @@ def _configure_trafficserver(self) -> None:
         ts.Disk.traffic_out.Content += Testers.ExcludesExpression(
             'use-after-free|attempting free|SEGV|received signal', 'ATS must survive the reject churn')
 
-    def _configure_client(self, tr) -> None:
+    def _configure_client(self, tr: 'TestRun') -> None:
         ts = self._ts
         client = os.path.join(Test.TestDirectory, 'rate_limit_sni_reject_client.sh')
         tr.Processes.Default.Command = f'bash {client} 127.0.0.1 {ts.Variables.ssl_port} rate.limited.com'

From 313a70fba40ece82d6f721812ffe20772d46def4 Mon Sep 17 00:00:00 2001
From: Mo Chen <mochen@apache.org>
Date: Tue, 4 Aug 2026 17:33:08 -0500
Subject: [PATCH 4/5] Deliver VCONN_CLOSE for connections parked in a TLS
 handshake hook

callHooks() moves the hook state to DONE when a connection closes, but it
kept curHook pointing into whichever handshake hook list the connection
was parked in. Each hook id owns a separate list, so advancing curHook
walked the handshake list rather than the close list: the close event was
dropped once that list ran out, and delivered to the next handshake
plugin when it did not.

A plugin that parks a connection therefore never learns that it died. In
the rate_limit SNI queue that leaves a freed TSVConn on the queue and
leaks the selector lease, and the next sweep reenables freed memory.

Restart from the head of the close hook list unless we are already
iterating it. Take the same path for TS_EVENT_VCONN_OUTBOUND_CLOSE, which
previously invoked nothing at all for a connection parked in the outbound
pre-handshake hook.
---
 src/iocore/net/TLSEventSupport.cc             | 12 ++-
 .../tls_hooks_close_while_parked.test.py      | 88 +++++++++++++++++++
 2 files changed, 99 insertions(+), 1 deletion(-)
 create mode 100644 tests/gold_tests/tls_hooks/tls_hooks_close_while_parked.test.py

diff --git a/src/iocore/net/TLSEventSupport.cc b/src/iocore/net/TLSEventSupport.cc
index c1009d8c6ca..a44d5c71b6c 100644
--- a/src/iocore/net/TLSEventSupport.cc
+++ b/src/iocore/net/TLSEventSupport.cc
@@ -153,10 +153,20 @@ TLSEventSupport::callHooks(TSEvent eventId)
   Dbg(dbg_ctl_ssl, "sslHandshakeHookState=%s eventID=%d", get_ssl_handshake_hook_state_name(this->sslHandshakeHookState), eventId);
 
   // Move state if it is appropriate
-  if (eventId == TS_EVENT_VCONN_CLOSE) {
+  if (eventId == TS_EVENT_VCONN_CLOSE || eventId == TS_EVENT_VCONN_OUTBOUND_CLOSE) {
     // Regardless of state, if the connection is closing, then transition to
     // the DONE state. This will trigger us to call the appropriate cleanup
     // routines.
+    //
+    // A connection can close while it is parked in a handshake hook, waiting for a plugin to
+    // reenable it. curHook then still points into that handshake hook's list. Each hook id owns
+    // a separate list, so advancing curHook below would walk the handshake list rather than the
+    // close list: the close event is dropped when the handshake list is exhausted, or delivered
+    // to the wrong plugin when it is not. Restart from the head of the close list unless we are
+    // already iterating it.
+    if (this->sslHandshakeHookState != SSLHandshakeHookState::HANDSHAKE_HOOKS_DONE) {
+      this->curHook = nullptr;
+    }
     this->sslHandshakeHookState = SSLHandshakeHookState::HANDSHAKE_HOOKS_DONE;
   } else {
     switch (this->sslHandshakeHookState) {
diff --git a/tests/gold_tests/tls_hooks/tls_hooks_close_while_parked.test.py b/tests/gold_tests/tls_hooks/tls_hooks_close_while_parked.test.py
new file mode 100644
index 00000000000..89fe0a50a3f
--- /dev/null
+++ b/tests/gold_tests/tls_hooks/tls_hooks_close_while_parked.test.py
@@ -0,0 +1,88 @@
+'''
+Verify that a plugin's VCONN_CLOSE hook runs when the connection closes while it is
+parked in a TLS handshake hook.
+'''
+#  Licensed to the Apache Software Foundation (ASF) under one
+#  or more contributor license agreements.  See the NOTICE file
+#  distributed with this work for additional information
+#  regarding copyright ownership.  The ASF licenses this file
+#  to you under the Apache License, Version 2.0 (the
+#  "License"); you may not use this file except in compliance
+#  with the License.  You may obtain a copy of the License at
+#
+#      http://www.apache.org/licenses/LICENSE-2.0
+#
+#  Unless required by applicable law or agreed to in writing, software
+#  distributed under the License is distributed on an "AS IS" BASIS,
+#  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+#  See the License for the specific language governing permissions and
+#  limitations under the License.
+
+import os
+
+Test.Summary = '''
+A connection that closes while parked in a TLS handshake hook must still deliver
+TS_VCONN_CLOSE_HOOK to the plugin.
+'''
+
+Test.SkipUnless(Condition.HasOpenSSLVersion("1.1.1"),)
+
+ts = Test.MakeATSProcess("ts", enable_tls=True)
+server = Test.MakeOriginServer("server")
+server.addResponse(
+    "sessionlog.json", {
+        "headers": "GET / HTTP/1.1\r\nHost: www.example.com\r\n\r\n",
+        "timestamp": "1469733493.993",
+        "body": ""
+    }, {
+        "headers": "HTTP/1.1 200 OK\r\nConnection: close\r\n\r\n",
+        "timestamp": "1469733493.993",
+        "body": ""
+    })
+
+ts.addDefaultSSLFiles()
+
+ts.Disk.records_config.update(
+    {
+        'proxy.config.diags.debug.enabled': 1,
+        'proxy.config.diags.show_location': 0,
+        'proxy.config.diags.debug.tags': 'ssl_hook_test',
+        # Fire the handshake timeout while the plugin still has the handshake parked (2s park).
+        'proxy.config.ssl.handshake_timeout_in': 1,
+        'proxy.config.ssl.server.cert.path': '{0}'.format(ts.Variables.SSLDir),
+        'proxy.config.ssl.server.private_key.path': '{0}'.format(ts.Variables.SSLDir),
+    })
+
+ts.Disk.ssl_multicert_yaml.AddLines(
+    """
+ssl_multicert:
+  - dest_ip: "*"
+    ssl_cert_name: server.pem
+    ssl_key_name: server.key
+""".split("\n"))
+
+ts.Disk.remap_config.AddLine(
+    'map https://example.com:{1} http://127.0.0.1:{0}'.format(server.Variables.Port, ts.Variables.ssl_port))
+
+# The delayed client hello callback parks the handshake for 2 seconds before it reenables.
+Test.PrepareTestPlugin(os.path.join(Test.Variables.AtsTestPluginsDir, 'ssl_hook_test.so'), ts, '-client_hello=1 -close=1')
+
+# Give up after 1 second, which is inside the 2 second park, so the connection closes while it
+# is still suspended in the client hello hook. curl reports operation timed out (exit 28).
+tr = Test.AddTestRun("Client disconnects while parked in the client hello hook")
+tr.Processes.Default.StartBefore(server)
+tr.Processes.Default.StartBefore(Test.Processes.ts)
+tr.StillRunningAfter = ts
+tr.StillRunningAfter = server
+tr.MakeCurlCommand('-k --max-time 1 -H \'host:example.com:{0}\' https://127.0.0.1:{0}'.format(ts.Variables.ssl_port), ts=ts)
+tr.Processes.Default.ReturnCode = 28
+tr.Processes.Default.TimeOut = 15
+tr.TimeOut = 15
+
+# The handshake really was parked.
+ts.Disk.traffic_out.Content = Testers.ContainsExpression("Client Hello callback 0", "the handshake parked in the client hello hook")
+
+# The close hook must still fire, with the correct event. Before the fix, callHooks() advanced
+# curHook within the client hello hook list instead of the close hook list, so this never ran.
+ts.Disk.traffic_out.Content += Testers.ContainsExpression(
+    "Close callback 0 .* - event is good", "the close hook ran for the parked connection")

From b1756013b2e184f340ecd0e353664df525fb80bc Mon Sep 17 00:00:00 2001
From: Mo Chen <mochen@apache.org>
Date: Thu, 6 Aug 2026 09:21:10 -0500
Subject: [PATCH 5/5] rate_limit: address review feedback

Drop the dependency on coreutils "timeout", which is absent on macOS and made
the gold tests fail rather than skip there, and which was relied on for
fractional deadlines. A small sleep-and-kill helper replaces it. Also drop
-verify_quiet, which is redundant with -quiet and is not accepted by every
s_client implementation.

Take the element by const reference in RateLimiter::remove(), and record what
bounds the scan: the configured queue size, or connections_throttle when a
"queue" is given without a "size".

Correct the queue test's narration. It described the counter wrapping and the
probe aborting the server, which is what happened before 508c1bea26 fixed the
sweep's resume condition; the test now pins that fix rather than reproducing it.
---
 plugins/experimental/rate_limit/limiter.h     |  7 ++-
 .../rate_limit_sni_expiry_client.sh           | 17 +++++-
 .../rate_limit/rate_limit_sni_queue_client.sh | 57 ++++++++++++-------
 .../rate_limit_sni_reject_client.sh           | 17 +++++-
 4 files changed, 71 insertions(+), 27 deletions(-)

diff --git a/plugins/experimental/rate_limit/limiter.h b/plugins/experimental/rate_limit/limiter.h
index 74979ad1d91..038c295faf1 100644
--- a/plugins/experimental/rate_limit/limiter.h
+++ b/plugins/experimental/rate_limit/limiter.h
@@ -330,8 +330,13 @@ template <class T> class RateLimiter
   // Remove a still-queued element (e.g. a connection that closed before it was resumed).
   // Returns true if it was found in the queue, so the caller can tell a queued element
   // (which never reserved a slot) from one that was already resumed.
+  //
+  // Linear in the queue depth, and only reached when an element closes while queued, which
+  // requires the limiter to be at its limit. The depth is bounded by the configured queue size;
+  // note that a "queue" without a "size" leaves _max_queue at UINT32_MAX, in which case the only
+  // bound is proxy.config.net.connections_throttle.
   bool
-  remove(T elem)
+  remove(const T &elem)
   {
     std::lock_guard<std::mutex> lock(_queue_lock);
 
diff --git a/tests/gold_tests/pluginTest/rate_limit/rate_limit_sni_expiry_client.sh b/tests/gold_tests/pluginTest/rate_limit/rate_limit_sni_expiry_client.sh
index 73fe04b62c8..10742f2c8c7 100644
--- a/tests/gold_tests/pluginTest/rate_limit/rate_limit_sni_expiry_client.sh
+++ b/tests/gold_tests/pluginTest/rate_limit/rate_limit_sni_expiry_client.sh
@@ -35,7 +35,20 @@ host="$1"
 port="$2"
 sni="$3"
 
-OSSL="openssl s_client -connect ${host}:${port} -servername ${sni} -quiet -verify_quiet -no_ign_eof"
+OSSL="openssl s_client -connect ${host}:${port} -servername ${sni} -quiet -no_ign_eof"
+
+# Run a command in the background and terminate it after a deadline. coreutils "timeout" is not
+# available everywhere (notably macOS), so do it with sleep and kill.
+run_for() {
+  deadline="$1"
+  shift
+  "$@" &
+  target=$!
+  (
+    sleep "${deadline}"
+    kill -TERM "${target}" 2>/dev/null
+  ) &
+}
 
 # 1. Holder: hold the single slot. Its stdin is a FIFO on fd 3 so we end it in step 4.
 fifo_dir="$(mktemp -d "${TMPDIR:-/tmp}/rl_holder.XXXXXX")"
@@ -57,7 +70,7 @@ exec 3>&-
 sleep 2
 
 # 5. Probe: its reserve() reads the counter; if it wrapped, the release assertion aborts.
-timeout 2 ${OSSL} </dev/null >/dev/null 2>&1 || true
+run_for 2 sh -c "${OSSL} </dev/null >/dev/null 2>&1"
 kill "${queued}" 2>/dev/null || true
 sleep 1
 
diff --git a/tests/gold_tests/pluginTest/rate_limit/rate_limit_sni_queue_client.sh b/tests/gold_tests/pluginTest/rate_limit/rate_limit_sni_queue_client.sh
index f8269cf8b7b..0ce7e56f3b1 100644
--- a/tests/gold_tests/pluginTest/rate_limit/rate_limit_sni_queue_client.sh
+++ b/tests/gold_tests/pluginTest/rate_limit/rate_limit_sni_queue_client.sh
@@ -16,19 +16,19 @@
 #  See the License for the specific language governing permissions and
 #  limitations under the License.
 #
-# Deterministically drive the rate_limit SNI limiter's queue-accounting balance bug. A
-# queued connection never increments the active-slot counter, but its VCONN_CLOSE always
-# decrements it, so one queued connection that closes is a single unmatched decrement.
-# With exactly one queued connection there is no way for the sweep to mask it:
+# Drive the rate_limit SNI limiter's queue-then-resume path with exactly one queued connection,
+# and check that the active-slot counter stays balanced and the server survives.
 #
 #   1. holder completes its handshake and holds the single slot (counter = 1);
-#   2. one connection enqueues (slot full), then closes cleanly (FIN) while parked;
-#   3. the sweep resumes it, its handshake fails and it closes -> one unmatched decrement
-#      -> counter 1 -> 0 (the queue is now empty, so no reserve() can rebalance it);
-#   4. the holder is closed; its matched decrement lands on the understated counter
-#      -> counter 0 -> wraps below zero;
-#   5. a probe connection's reserve() observes the wrapped counter and the limiter's
-#      release assertion (_active <= _limit) aborts the server.
+#   2. one connection enqueues because the slot is full, then closes while parked;
+#   3. the sweep reserves a slot and resumes a queued connection;
+#   4. the holder is closed and releases its slot;
+#   5. a probe connection reserves the freed slot.
+#
+# Against the plugin before 508c1bea26 this aborts the server: the sweep resumed a queued
+# connection without a reservation, whose close then decremented the counter unmatched until it
+# wrapped and reserve() tripped TSReleaseAssert(_active <= _limit). The test asserts the counter
+# never wraps and no signal is logged, so it pins that fix as well as this change.
 #
 # args: host port sni
 set -u
@@ -36,7 +36,20 @@ host="$1"
 port="$2"
 sni="$3"
 
-OSSL="openssl s_client -connect ${host}:${port} -servername ${sni} -quiet -verify_quiet -no_ign_eof"
+OSSL="openssl s_client -connect ${host}:${port} -servername ${sni} -quiet -no_ign_eof"
+
+# Run a command in the background and terminate it after a deadline. coreutils "timeout" is not
+# available everywhere (notably macOS), so do it with sleep and kill.
+run_for() {
+  deadline="$1"
+  shift
+  "$@" &
+  target=$!
+  (
+    sleep "${deadline}"
+    kill -TERM "${target}" 2>/dev/null
+  ) &
+}
 
 # 1. Holder: hold the single slot. Its stdin is a FIFO kept open on fd 3, so we end the
 #    holder deterministically in step 4 (closing fd 3 -> EOF -> clean TLS close -> FIN).
@@ -48,18 +61,18 @@ exec 3<>"$fifo"
 rm -rf "$fifo_dir"
 sleep 3 # let the holder reserve the one slot
 
-# 2. One queued connection: enqueues (slot full), then sends a clean FIN ~0.3s later while
-#    still parked at the ClientHello hook.
-timeout 0.3 ${OSSL} </dev/null >/dev/null 2>&1 &
-sleep 2 # >= 2 sweep periods (300ms each): the sweep resumes the queued connection, its
-        # handshake fails (EPIPE) and it closes -> one unmatched decrement -> counter 1 -> 0
+# 2. One queued connection: enqueues because the slot is full, then closes while still parked
+#    at the ClientHello hook.
+run_for 0.3 sh -c "${OSSL} </dev/null >/dev/null 2>&1"
+sleep 2 # >= 2 sweep periods (300ms each), so the sweep runs while the connection is queued
 
-# 4. End the holder: its matched decrement lands on the already-understated counter.
+# 4. End the holder, releasing its slot.
 exec 3>&- # close the FIFO write end -> holder sees EOF -> clean TLS close (FIN)
-sleep 2   # let the holder's close run: counter 0 -> wraps below zero
+sleep 2
 
-# 5. Probe: its reserve() reads the wrapped counter and trips TSReleaseAssert(_active <= _limit).
-timeout 2 ${OSSL} </dev/null >/dev/null 2>&1 || true
-sleep 1
+# 5. Probe: reserve() must succeed against a balanced counter rather than tripping the
+#    TSReleaseAssert(_active <= _limit) that a wrapped counter causes.
+run_for 2 sh -c "${OSSL} </dev/null >/dev/null 2>&1"
+sleep 3
 
 echo "rate_limit-queue-crash-done"
diff --git a/tests/gold_tests/pluginTest/rate_limit/rate_limit_sni_reject_client.sh b/tests/gold_tests/pluginTest/rate_limit/rate_limit_sni_reject_client.sh
index bd1f8e07b6f..8d44b314bcd 100644
--- a/tests/gold_tests/pluginTest/rate_limit/rate_limit_sni_reject_client.sh
+++ b/tests/gold_tests/pluginTest/rate_limit/rate_limit_sni_reject_client.sh
@@ -29,7 +29,20 @@ host="$1"
 port="$2"
 sni="$3"
 
-OSSL="openssl s_client -connect ${host}:${port} -servername ${sni} -quiet -verify_quiet -no_ign_eof"
+OSSL="openssl s_client -connect ${host}:${port} -servername ${sni} -quiet -no_ign_eof"
+
+# Run a command in the background and terminate it after a deadline. coreutils "timeout" is not
+# available everywhere (notably macOS), so do it with sleep and kill.
+run_for() {
+  deadline="$1"
+  shift
+  "$@" &
+  target=$!
+  (
+    sleep "${deadline}"
+    kill -TERM "${target}" 2>/dev/null
+  ) &
+}
 
 # holder: complete the handshake and hold the single slot for ~5s (slow stdin keeps it open).
 (sleep 5) | ${OSSL} >/dev/null 2>&1 &
@@ -38,7 +51,7 @@ sleep 2 # let the holder reserve the slot
 # Burst of near-simultaneous handshakes against the full limiter; with no queue every one
 # is rejected with TS_EVENT_ERROR, so its handshake VC is torn down consumer-driven.
 for _ in $(seq 5); do
-  timeout 2 ${OSSL} </dev/null >/dev/null 2>&1 &
+  run_for 2 sh -c "${OSSL} </dev/null >/dev/null 2>&1"
 done
 
 # Let the burst finish and the holder release its slot cleanly.