bryancall commented on PR #13661:
URL: https://github.com/apache/trafficserver/pull/13661#issuecomment-5648166444
Superseded by #13671, which carries both commits from this PR as its first
two, plus two
more defects in the same file.
Closing rather than updating for one reason. This PR bundled two
backportable fixes (the
shared-context constructor and the pthread-key JIT stack) with a
source-breaking change
(deleting `RegexMatchContext`'s copy and move members). Since a merged PR
becomes a single
squashed commit, that mix would have kept the backportable half off 10.2.x
and 10.1.x: a
release manager cherry-picking it would have taken the source break along
with it. #13671
is purely behavioural with no change to the public API surface, so it
carries a Backport
label and can travel as a unit.
The member deletion is not lost, it is deferred to #13663, where
`RegexMatchContext` is
replaced by a value type and the question of deleting its members stops
being a source
break in isolation.
Two things from the review here that carried over to #13671:
- **The `pthread_key_create` failure path.** Your point stands and the
handling added in
`fff1a840a2` is in #13671 unchanged, along with the additional case where
`pthread_setspecific` fails, which would otherwise have leaked a stack per
match.
- **The `thread_local` hazard was only half addressed here.** This PR moved
the JIT stack
to a pthread key, but the general, compile and match contexts were
themselves in a
`thread_local` with a destructor, so the first `compile()` or `exec()` on
a thread still
registered through `__cxa_thread_atexit` and still took the loader lock.
#13671 makes
those contexts process-wide, which is what actually closes it. The
sentence in this PR's
description claiming the hazard is gone was overstated, and that
correction is the reason
the follow-up exists.
#13671 also adds the eight-thread concurrency test this file never had, and
is clean under
both ThreadSanitizer and AddressSanitizer.
--
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]
