Inherit the shared match context configuration in RegexMatchContext

[bryancall]

RegexMatchContext builds an empty PCRE2 match context, so a caller that supplies its
own context silently loses everything the shared context configures. Today that is a
1 MB JIT stack, replaced by PCRE2's 32 KB fallback. regex_remap and esi are the only
two users of the type, and both ran on the fallback.

Nobody chose 32 KB. It is what PCRE2 uses when no stack is assigned, and it arrived by
omission when the type was introduced in #12575. That shipped in 10.2.0.

This builds the context as a copy of the shared one instead, so a caller cannot silently
lack what the shared context has, and anything added to the shared context later applies
automatically. There is no change to regex_remap.cc; the plugin keeps its context and
the context now starts correct.

Issue: #13660

What this changes in behaviour

Measured with pcre2test 10.47 against the rule and the 3071 byte URL already in
replay/yts-2819.replay.json:

~^/alpha/bravo/[?]((?!action=(newsfeed|calendar|contacts|notepad)).)*$~
  jitstack=32     Failed: error -46: JIT stack limit reached
  jitstack=1024   matches

That URL now redirects rather than falling through to origin, which is what the rule
always intended. The first -46 threshold for this rule moves from 1377 bytes to 43702
bytes, which is past the 32768 byte default of
proxy.config.http.request_header_max_size. So after this change the request size limit
binds before the JIT stack does, which is where that bound belongs.

The 32 KB fallback lives on the machine stack. An assigned stack is heap allocated. So
this reduces thread stack pressure rather than raising it, which is worth stating because
the limit this rule has been hitting traces back to #5762, where the concern was machine
stack exhaustion.

1 MB is not a new number. It is what ATS has used since #11014, and the same 4 KB floor
and 1 MB ceiling the pre-PCRE2 code used before that.

Why the stack is supplied through a callback

regex_remap.cc:810 holds one RegexMatchContext per remap instance, built once at
config load and used by every ET_NET thread. One object, many threads.

Assigning a stack pointer to that context would give every thread the same stack:

flowchart LR
    CTX["one RegexMatchContext<br/>shared by every thread"] --> T0["ET_NET 0"]
    CTX --> T1["ET_NET 1"]
    CTX --> T2["ET_NET 2"]
    T0 --> S["one JIT stack"]
    T1 --> S
    T2 --> S
    S --> X["concurrent matches<br/>corrupt it"]
    style CTX fill:#F1EFE8,stroke:#5F5E5A,color:#2C2C2A
    style S fill:#FCEBEB,stroke:#A32D2D,color:#501313
    style X fill:#FCEBEB,stroke:#A32D2D,color:#501313

Loading

pcre2jit is explicit: "if you assign or pass back a non-NULL JIT stack, this must be a
different stack for each thread so that the application is thread-safe."

So the context carries a callback instead. PCRE2 invokes it at match time, on the thread
that is matching, and it returns that thread's own stack:

flowchart LR
    CTX["one RegexMatchContext<br/>shared by every thread"] --> T0["ET_NET 0"]
    CTX --> T1["ET_NET 1"]
    CTX --> T2["ET_NET 2"]
    T0 --> S0["stack for thread 0"]
    T1 --> S1["stack for thread 1"]
    T2 --> S2["stack for thread 2"]
    style CTX fill:#F1EFE8,stroke:#5F5E5A,color:#2C2C2A
    style S0 fill:#E1F5EE,stroke:#0F6E56,color:#04342C
    style S1 fill:#E1F5EE,stroke:#0F6E56,color:#04342C
    style S2 fill:#E1F5EE,stroke:#0F6E56,color:#04342C

Loading

This is the arrangement the pre-PCRE2 code used, and the pattern the pcre2jit manual
recommends. Note the sharing is a property of one context being used by many threads, not
of the type being copyable: the callback is still required with the copy and move members
deleted, as they now are.

Verified under ThreadSanitizer: eight threads, two thousand matches each, one shared
context, clean. Cost is below noise, 19.9 to 25.4 ns per match assigning a stack directly
against 20.4 to 21.7 ns through the callback, on a short ordinary match.

Also deletes the copy and move members

Nothing in the tree copies or moves a RegexMatchContext. Every use is a plain member or
local: regex_remap.cc:810, IncludeUrlValidator.h:83, and two unit tests. The four
special members were dead code carrying two defects:

• The defaulted move copied the raw pointer and left the source holding it, so both
  destructors called pcre2_match_context_free on the same object.
• The copy constructor left the pointer null when the source was null, which its own
  destructor asserts on in a debug build and silently ignores in a release build.

They are now = delete, which makes both unrepresentable rather than fixing them. Neither
had ever fired, because nothing exercises them. This removes 24 lines of dead definitions
from Regex.cc.

The per thread stack uses a pthread key, not a thread local

A thread_local with a destructor registers it through __cxa_thread_atexit, which takes
the dynamic loader lock. Registering that from Regex::exec inverts lock order against a
dlopen caller running a plugin's static initialization. Diags::tag_activated already
documents this hazard and works around it, and the pre-PCRE2 implementation of this file
worked around it too, with the arrangement lost in #11014.

Earlier revisions of this branch reproduced the whole trade: a thread local holding the
stack deadlocked, splitting the pointer from the cleanup object leaked the stack instead,
and arming the cleanup from exec only moved the registration out of PCRE2's callback
while leaving it on exec, and extended it to the caller-supplied context path that never
had it.

The stack now lives in a pthread_key_t whose destructor is registered once at key
creation through pthread_once and never from the matching path. That removes both
thread locals, the arming function and its call, so the hazard is gone rather than
balanced.

The #5762 crash guard is preserved, not removed

The AuTest run that guards #5762 previously used the 3071 byte URL. Since that URL now
matches, the run would no longer have exercised a resource limit. Rather than drop the
guard, the run keeps its shape at a subject large enough to still exhaust 1 MB, and the
request header limit is raised for it. Verified in the sandbox:

Bad regular expression result -46 ("JIT stack limit reached") from "^/alpha/bravo/..."
Bad regular expression result -47 ("match limit exceeded") from "^/match_limit/(a+)+$"

Two unit tests are added in test_Regex.cc: one asserting a caller-supplied context
reaches the same verdict as the shared one, and one asserting that resource exhaustion is
reported rather than crashing. The second asserts directly the property #5762 was
protecting, which the AuTest can only reach indirectly.

Both ask PCRE2 for the pattern's JIT size and skip when there is none, because both are
about the JIT stack and PCRE2 consults it only when it has JIT code to run. Without JIT a
blank context and the shared one both take the interpreter and return the same answer, so
the parity test would pass whether or not the fix were present, and the exhaustion test
would fail outright: the interpreter keeps its backtracking frames on the heap and matches
a 2 MiB subject rather than running out of anything. Measured, not assumed.

Testing

On a dev-asan build:

• Full ctest suite: 134 of 134 pass. This touches shared tsutil code, so the whole
  suite is the relevant blast radius rather than the Regex tests alone.
• regex_remap and regex_remap_long_query AuTests pass, with the 3 KB URL returning
  301 and exactly one -46 and one -47 in the diags log.

AuTest flakes seen while testing this, both pre-existing

Neither is introduced by this change. Recording them because both cost time to rule out.

connect_attempts failed once on this PR in CI and passed on a retrigger of the
identical commit. The gold file pins the HTTP state machine id, which is an ordering
artifact rather than the retry behaviour under test. Filed as #13664. That test does not
use regex, and it passed five out of five locally on this branch.

regex_remap is nondeterministic under an ASAN build because LeakSanitizer reports a
104 byte leak from ConfigReloadTask::start_progress_checker() at
src/mgmt/config/ConfigReloadTrace.cc:394, which fails the traffic_server exit code.
Filed as #13662. This one does not affect CI, because ci-fedora-autest does not enable
ASAN; it shows up only in a local dev-asan build.

Out of scope

Whether regex_remap should carry a CPU bound at all, and what it should be, is a
separate question from whether a caller-supplied context should silently differ from the
shared one. The remaining design problems with this type, including replacing it with a
value, are #13663. Related: #13654.

[bryancall]

[approve ci autest 0]

[bryancall]

[approve ci clang-analyzer freebsd autest 1 autest 2]

[Copilot]

🔵 Needs a closer look

It changes shared regex matching behavior and thread-local JIT stack handling in core tsutil code, which warrants final human validation despite tests and careful design.

Pull request overview

This PR updates RegexMatchContext so that a caller-supplied match context inherits the shared PCRE2 match-context configuration (notably the 1 MiB JIT stack behavior) instead of silently falling back to PCRE2 defaults. This aligns regex_remap and esi behavior with the rest of ATS and prevents unintended JIT-stack-limit failures caused by an “empty” context.

Changes:

• Build RegexMatchContext by copying the shared match context, so shared configuration is preserved automatically.
• Switch the shared match context to use a per-thread JIT stack via a PCRE2 callback, and ensure safe TLS cleanup is armed from Regex::exec.
• Update the regex_remap AuTest expectations and add unit tests that assert shared-vs-supplied-context parity and resource-exhaustion behavior.

File summaries

File	Description
src/tsutil/Regex.cc	Uses a PCRE2 JIT-stack callback (per-thread stack) and arms TLS cleanup before matching; RegexMatchContext now copies the shared match context.
include/tsutil/Regex.h	Deletes copy/move operations for RegexMatchContext to prevent unsafe ownership semantics.
src/tsutil/unit_tests/test_Regex.cc	Adds unit tests covering context parity and resource-exhaustion (non-crash) behavior under JIT.
tests/gold_tests/pluginTest/regex_remap/regex_remap.test.py	Updates AuTest to reflect redirect behavior for the ~3 KiB URL and preserves the original crash guard with a larger request/header limit.

Review details

• Files reviewed: 4/4 changed files
• Comments generated: 1
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

🟡 Changes recommended

The critical loader-lock hazard remains unresolved, and deleting public copy/move members is source-breaking.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (1)

Previously missed (1) — in code that hasn't changed since the last review.

include/tsutil/Regex.h:122

• Deleting these public special members is a source-breaking change: Regex.h is listed in TSUTIL_PUBLIC_HEADERS and installed by src/tsutil/CMakeLists.txt:34,60-71, while the previous copy constructor was documented as a deep copy. Code outside this tree that copies RegexMatchContext (or a containing type such as IncludeUrlValidator) will no longer compile; preserve correct copy/move implementations or explicitly make this an API-breaking release change.

• Files reviewed: 4/4 changed files
• Comments generated: 1
• Review effort level: Lite

[Copilot]

🟡 Changes recommended

Five moderate findings remain unresolved.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (4)

Previously missed (1) — in code that hasn't changed since the last review.

include/tsutil/Regex.h:122

• Regex.h is installed as a TSUTIL_PUBLIC_HEADERS header, and the copy constructor was previously documented as a supported deep copy. Deleting all four special members is therefore a source/ABI breaking change for downstream users even though the repository has no current call sites; retain safe copy/move operations (or provide an explicit compatibility/deprecation transition) rather than removing the public operations without notice.

src/tsutil/unit_tests/test_Regex.cc:1072

• Both a pattern compilation failure and a JIT compilation failure are collapsed into false, so either malformed test pattern would cause the test to SKIP before the Regex::compile assertion. This can turn a broken regression test into a green skip; distinguish the no-JIT case from actual PCRE2 errors and fail the test for the latter.

  pcre2_code *code = pcre2_compile(reinterpret_cast<PCRE2_SPTR>(pattern), PCRE2_ZERO_TERMINATED, 0, &errnum, &erroffset, nullptr);
  if (code == nullptr) {
    return false;
  }
  pcre2_jit_compile(code, PCRE2_JIT_COMPLETE);

src/tsutil/unit_tests/test_Regex.cc:1130

• The portability rationale here is backwards: with a fixed 1 MiB JIT stack, smaller JIT frames increase the subject length needed to exhaust it, while larger frames make exhaustion happen sooner. On a platform whose threshold exceeds 256 KiB, the test will return RE_ERROR_NOMATCH and fail even though the resource-exhaustion guard is working; size this from a documented cross-platform bound or correct the claim and establish that bound.

  // This pattern starts failing at roughly 43KiB of subject against a 1MiB JIT
  // stack, measured identically on x86_64 and arm64. 256KiB keeps a six times
  // margin for a platform whose JIT frames are larger, without allocating more
  // than the bound needs. Do not trim this to just above 43KiB.

src/tsutil/unit_tests/test_Regex.cc:1107

• This test does not make the regression observable: both calls use a 1000-character subject and only require the caller-supplied context to return the same successful result. A blank context can still produce that result whenever the subject stays below PCRE2's 32 KiB fallback, so this test would pass before the context-copy fix. Use a pattern/subject pair that is known to exhaust the fallback while succeeding with the shared 1 MiB stack, or otherwise assert a differential outcome tied to the two stack sizes.

  std::string const subject(1000, 'a');

  RegexMatches      shared_matches;
  RegexMatchContext match_context;
  RegexMatches      own_matches;

  int const shared_rc = re.exec(subject, shared_matches);
  int const own_rc    = re.exec(subject, own_matches, 0, &match_context);
  CAPTURE(shared_rc, own_rc);

  REQUIRE(shared_rc > 0);
  REQUIRE(own_rc == shared_rc);

• Files reviewed: 4/4 changed files
• Comments generated: 1
• Review effort level: Lite

[Copilot]

🔵 Needs a closer look

Unresolved moderate findings affect public API compatibility, stack cleanup, and concurrency coverage.

Review details

Suppressed comments (4)

Previously missed (1) — in code that hasn't changed since the last review.

include/tsutil/Regex.h:122

• Regex.h is installed as a public tsutil header (src/tsutil/CMakeLists.txt:18-35,60-71), so deleting all copy/move operations is a source-breaking API change for downstream users even though in-tree call sites do not use them. The old operations were unsafe, but this also prevents otherwise valid uses such as returning or storing a context in a movable container; preserve correct deep-copy/move semantics or explicitly handle this as a versioned API break.

src/tsutil/Regex.cc:126

• If pthread_setspecific fails, the newly allocated stack is still returned to PCRE2 but is not retained in TLS. Every later match on that thread then allocates another stack and leaks the previous one, turning a recoverable resource failure into an unbounded per-match leak. Check the return value and free the stack before falling back to a null callback result.

    pthread_setspecific(jit_stack_key, stack);

src/tsutil/Regex.cc:175

• The new callback is the concurrency-critical part of the fix: one RegexMatchContext may be shared by multiple matching threads, and each must receive a distinct JIT stack. The added tests exercise only serial matches, so a regression to a single shared stack would still pass; add an automated test that matches concurrently through one context (ideally under the existing thread-safety/TSan coverage).

    pcre2_jit_stack_assign(_match_context, jit_stack_for_this_thread, nullptr);

src/tsutil/unit_tests/test_Regex.cc:1140

• This only proves that the match returned some error other than RE_ERROR_NOMATCH; an unrelated failure such as RE_ERROR_NULL would also satisfy the guard. Since pattern_has_jit() gates this test and the subject is intended to exhaust the shared JIT stack, assert PCRE2_ERROR_JIT_STACKLIMIT specifically so a broken match path cannot make the regression test pass.

  REQUIRE(rc < 0);
  REQUIRE(rc != RE_ERROR_NOMATCH);

• Files reviewed: 4/4 changed files
• Comments generated: 0 new
• Review effort level: Lite