bryancall commented on code in PR #13661: URL: https://github.com/apache/trafficserver/pull/13661#discussion_r3983952779
########## src/tsutil/Regex.cc: ########## @@ -499,6 +514,8 @@ Regex::exec(std::string_view subject, RegexMatches &matches, uint32_t flags, Reg bool const full_match = (flags & RE_FULL_MATCH) != 0; uint32_t const pcre2_flags = flags & ~RE_FULL_MATCH; + arm_jit_stack_cleanup(); Review Comment: Good catch, and the citation checks out. `Diags::tag_activated` documents exactly this hazard and works around it: > exec() can lazily construct a thread_local RegexContext whose destructor registration > via __cxa_thread_atexit_impl takes the dynamic loader lock. Holding tag_table_lock > across exec() therefore creates a lock-order inversion with dlopen() callers that > construct a DbgCtl during a plugin's static initialization. You are right that arming before the match only moved the registration out of PCRE2's callback and left it on `exec`. Worse, it put it on the caller-supplied context path, which previously never touched a thread local with a destructor at all, so this change was extending the hazard rather than containing it. Fixed in 143e53238f by taking the C++ thread local out entirely. The per thread 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 also deletes the arming function, its call in `exec`, and both thread locals, so the deadlock and leak trade this had been balancing goes away instead: 24 insertions against 27 deletions. Verified on a dev-asan build: unit suite 134 of 134, the regex_remap AuTest passes with the 3KB URL returning 301 and one -46 plus one -47 in the diags log, and LeakSanitizer reports no `jit_stack_for_this_thread` allocation, so the key's destructor is running. -- 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]
