peak_ewma: fix segfault from timer thread-safety violation#43526
Open
rroblak wants to merge 1 commit intoenvoyproxy:mainfrom
Open
peak_ewma: fix segfault from timer thread-safety violation#43526rroblak wants to merge 1 commit intoenvoyproxy:mainfrom
rroblak wants to merge 1 commit intoenvoyproxy:mainfrom
Conversation
rroblak
force-pushed
the
rroblak/fix-peak-ewma-segfault
branch
from
a6c4dfb to
2831f68
Compare
Contributor
|
Thanks @rroblak for writing this fix! CI / build is currently failing. |
rroblak
force-pushed
the
rroblak/fix-peak-ewma-segfault
branch
from
2831f68 to
5f07411
Compare
Contributor
Author
|
Thanks @frittentheke! I suspect the CI failure was due to an unrelated Go dependency resolution issue that was fixed by #43536. I've rebased and pushed— let's see if that clears it up. |
…y#43513) - Remove dispatcher/timer dependency; aggregate inline in chooseHost() - Remove destructor that cleared host lbPolicyData (race with workers) - Clean up all_host_stats_ on host removal (fix shared_ptr leak) - Remove dispatcher from config factory - Add host lifecycle regression tests for all three bugs Signed-off-by: Ryan Oblak <rroblak@gmail.com>
rroblak
force-pushed
the
rroblak/fix-peak-ewma-segfault
branch
from
5f07411 to
a9c0016
Compare
Contributor
Author
|
@frittentheke CI is green! Ready for your smoke test whenever you get a chance. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Commit Message: peak_ewma: fix segfault from timer thread-safety violation (#43513)
Additional Description:
Hopefully fixes #43513. The Peak EWMA LB constructor took an
Event::Dispatcher&and calledcreateTimer()on it. When instantiated on worker threads (via dynamic config such as Istio EnvoyFilter or Envoy Gateway EnvoyPatchPolicy), this violated Envoy's thread-safety model — timers must be created on the dispatcher's owning thread — causingassert failure: isThreadSafe()(debug) or segfault (release).This PR:
chooseHost(), removing theEvent::Dispatcher&dependency entirelylbPolicyData(raced with workers still reading)all_host_stats_entries on host removal (shared_ptr leak)Risk Level: Low — peak_ewma is a contrib extension; changes are isolated to its source and tests.
Testing: New
peak_ewma_lb_host_lifecycle_test.ccwith regression tests for all 3 bugs. All existing peak_ewma tests pass.Docs Changes: N/A
Release Notes: N/A
Platform Specific Features: N/A