bryancall commented on issue #13663:
URL:
https://github.com/apache/trafficserver/issues/13663#issuecomment-5624460308
Findings 1 and 2 are fixed in #13661 rather than here, because the fix
turned out to
be deletion rather than repair.
Nothing in the tree copies or moves a `RegexMatchContext`. Every use is a
plain member
or local: `plugins/regex_remap/regex_remap.cc:810`,
`plugins/esi/lib/IncludeUrlValidator.h:83`, and two unit tests. So all four
special
members were dead code, and both defects lived only in code nobody called.
They are now
`= delete`, which makes the double free and the null-copy unrepresentable
instead of
patched. Verified: the tree still builds, and the full unit suite passes.
That leaves this issue with the three design points, which stand on their
own:
- two nulls with two meanings and one spelling
- `my_malloc` and `my_free` as ceremony around a general context that
carries no
information
- the optional-pointer-with-null-sentinel parameter, on a type whose entire
public
surface is one setter
One correction to the framing above, which I had wrong. I described the JIT
stack
callback in #13661 as necessary because a copied context can cross threads.
That is not
the reason. `regex_remap` holds one context that every ET_NET thread matches
through, so
the sharing is there whether or not the type is copyable, and deleting the
copy members
does not remove the need for the callback.
That actually strengthens the case for the `Options` proposal. The problem
is not that
the context can be copied. It is that a caller owns a long lived context at
all.
--
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]
