dns cache: remove main thread assert in cache lookup #18301
Conversation
Signed-off-by: Mike Schore <mike.schore@gmail.com>
There was a problem hiding this comment.
@goaway @Augustyniak @Reflejo and I had a long discussion about this. We considered converting this to a KNOWN_ISSUE_ASSERT, but at @goaway points out the removal of this assertion preserves the existing contract in getCache above where it is up to the caller to call from the main thread -- which in that function's case is true due to only being called during config object construction (which always happens on the main thread).
There was a problem hiding this comment.
Per offline chat I'm OK removing this to unblock you, but I would like to ASSERT brought back after you fix whatever is causing this assert to fire. I don't understand how/why this code is getting called so early that this reference is invalid, especially since you must supply the factory context from the server itself.
@junr03 please open an issue to track and then follow up with a proper fix in the next few days. Thank you.
|
@mattklein123 agreed. Issue is here envoyproxy/envoy-mobile#1842 |
* main: (114 commits) kafka: add header support to mesh-filter (envoyproxy#18248) rbac: add support for upstream ip policy. (envoyproxy#17645) SIPProxy BUGFIX UT failure for fastbuild/debug (envoyproxy#18303) quic: updating goaway code (envoyproxy#18291) various tiny fixes (envoyproxy#18287) dns cache: remove assert at this layer (envoyproxy#18301) [ext_authz]: ext_authz filter unit test that use real threading (envoyproxy#17742) signal action: fully disable sigaltstack on Apple (envoyproxy#18299) Add missing dependencies (envoyproxy#18297) ext_proc: Pass stream_info to gRPC streams (envoyproxy#18190) use clang 12 (envoyproxy#18220) Update PR template to include the "Fixes commit" message when reverting or fixing bad commits (envoyproxy#18298) [test] Fixing integration test to cleanup cleanly (envoyproxy#18293) test: moving grpc bridge tests out of core directory (envoyproxy#18227) runtime: disable deprecated extensions names by default (envoyproxy#18239) quiche: updating deps (envoyproxy#18272) sip_proxy: SIP protocol support in envoy (envoyproxy#18039) http: add core retry policy to route retry policy conversion utility (envoyproxy#17803) build: updating stale visibility (envoyproxy#18278) alternate_protocols_cache: Impose a max size limit on the alternate protocols cache (envoyproxy#18258) ... Signed-off-by: Michael Puncel <mpuncel@squareup.com>
Commit Message: dns cache: remove assert at this layer
Additional Description: Envoy Mobile has logic that executes on the main thread but may not have the necessary state to pass this assertion. This change maintains the existing convention that callers of the DNS cache manager should validate their thread context independently.
Risk Level: Low
Testing: Local & CI
Signed-off-by: Mike Schore mike.schore@gmail.com