-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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
ext_proc: Stop timers during ExternalProcessing::Filter::onDestroy #22541
ext_proc: Stop timers during ExternalProcessing::Filter::onDestroy #22541
Conversation
If a timer fires after onDestroy is called, the ext_proc filter may crash while trying to access invalid encoder or decoder filter callbacks. The documentation for StreamFilterBase::onDestroy() says, "Filters must not invoke either encoder or decoder filter callbacks after having onDestroy() invoked." Signed-off-by: Rick Stewart <ristewart@google.com>
/assign @mpwarres @PiotrSikora |
You might also consider adding a test case to ext_proc_integration_test.cc similar to ExtProcIntegrationTest.ResponseMessageTimeout, but with the added element that the downstream disconnects before time is advanced. |
Signed-off-by: Rick Stewart <ristewart@google.com>
Done. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks! CI seems to be in a bad state, can you take a look?
Signed-off-by: Rick Stewart <ristewart@google.com>
/retest |
Retrying Azure Pipelines: |
/retest |
Retrying Azure Pipelines: |
…s_on_decode Signed-off-by: Rick Stewart <ristewart@google.com>
Thanks! I fixed the tests. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
Signed-off-by: Rick Stewart ristewart@google.com
Commit Message: If a timer fires after onDestroy is called, the ext_proc filter may crash while trying to access invalid encoder or decoder filter callbacks. The documentation for StreamFilterBase::onDestroy() says, "Filters must not invoke either encoder or decoder filter callbacks after having onDestroy() invoked." envoy-security@googlegroups.com said it's OK to upstream this since ext_proc is marked alpha.
Additional Description:
Risk Level: Low
Testing: Unit tests added
Docs Changes:
Release Notes:
Platform Specific Features:
[Optional Runtime guard:]
[Optional Fixes #Issue]
[Optional Fixes commit #PR or SHA]
[Optional Deprecated:]
[Optional API Considerations:]