Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

golang: avoid accessing deleted decoder_callbacks #37405

Merged
merged 2 commits into from
Dec 2, 2024
Merged

Conversation

spacewander
Copy link
Contributor

@spacewander spacewander commented Nov 28, 2024

This patch is provided by @doujiang24.

Commit Message: golang: avoid accessing deleted state
Additional Description: During Golang GC there will be envoyGoFilterHttpFinalize -> deferredDeleteRequest -> getDispatcher. Before this fix, the getDispatcher will use the occasional deleted decoder callback, which will cause a crash. After this fix, we won't touch the deleted callback anymore.
Risk Level: Low
Testing: after 3 hours load testing, the Envoy with this fix is still running well.
Docs Changes:
Release Notes:
Platform Specific Features:
[Optional Runtime guard:]
Fixes #37225
[Optional Fixes commit #PR or SHA]
[Optional Deprecated:]
[Optional API Considerations:]

Copy link

As a reminder, PRs marked as draft will not be automatically assigned reviewers,
or be handled by maintainer-oncall triage.

Please mark your PR as ready when you want it to be reviewed!

🐱

Caused by: #37405 was opened by spacewander.

see: more, trace.

@spacewander spacewander marked this pull request as ready for review November 28, 2024 12:08
@spacewander spacewander changed the title golang: avoid accessing deleted state golang: avoid accessing deleted decoder_callbacks Nov 28, 2024
Signed-off-by: spacewander <[email protected]>
@spacewander
Copy link
Contributor Author

The CI (https://github.com/envoyproxy/envoy/actions/runs/12078238750/job/33682570374) fails in "verify example - jaeger-tracing", which is not relative to this change.

@doujiang24
Copy link
Member

/retest

Copy link
Member

@doujiang24 doujiang24 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm, it's good to merge, thanks cc @RyanTheOptimist

@ggreenway ggreenway merged commit d33332a into envoyproxy:main Dec 2, 2024
24 checks passed
@phlax
Copy link
Member

phlax commented Dec 2, 2024

/backport

@repokitteh-read-only repokitteh-read-only bot added the backport/review Request to backport to stable releases label Dec 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport/review Request to backport to stable releases
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Envoy with golang filter crashes randomly
4 participants