Add support to unregister from lifecycle notifications #6984
Conversation
Signed-off-by: Elisha Ziskind <eziskind@google.com>
Signed-off-by: Elisha Ziskind <eziskind@google.com>
|
/review @htuch |
| public: | ||
| virtual ~Handle() = default; | ||
|
|
||
| virtual void unregister() PURE; |
There was a problem hiding this comment.
drive by: Can we just have destruction of the Handle unregister.
Bonus points: we have this pattern in quite a few different places, and I'm wondering if we could make this some type of generic mix-in class? WDYT?
There was a problem hiding this comment.
Moved unregister to the destructor.
Can you point me to some other examples of this pattern?
Signed-off-by: Elisha Ziskind <eziskind@google.com>
Signed-off-by: Elisha Ziskind <eziskind@google.com>
There was a problem hiding this comment.
@eziskind thanks, LGTM. I think in terms of other places where this patterns comes up, check out ClusterUpdateCallbacksHandleImpl and ConfigTrackerImpl::EntryOwnerImpl. Probably others as well. Do you want to generalize in this PR or leave this as a TODO?
Thanks, I'll leave this as a TODO for a followup PR. |
|
@eziskind LGTM if you can add the TODO or file an issue for the general case. |
* master: (65 commits) proto: Add PATCH method to RequestMethod enum (envoyproxy#6737) exe: drop unused deps on zlib compressor code (envoyproxy#7022) coverage: fix some misc coverage (envoyproxy#7033) Enable proto schema for router_check_tool (envoyproxy#6992) stats: rework stat sink flushing to centralize counter latching (envoyproxy#6996) [test] convert lds api test config stubs to v2 (envoyproxy#7021) router: scoped rds (2c): implement scoped rds API (envoyproxy#6932) build: Add option for size-optimized binary (envoyproxy#6960) test: adding an integration test framework for file-based LDS (envoyproxy#6933) doc: update obsolete ref to api/XDS_PROTOCOL.md (envoyproxy#7002) dispatcher: faster runOnAllThreads (envoyproxy#7011) example: add csrf sandbox (envoyproxy#6805) fix syntax of gcov exclusion zone. (envoyproxy#7023) /runtime_modify: add support for query params in body (envoyproxy#6977) stats: Create stats for http codes with the symbol table. (envoyproxy#6733) health check: fix more fallout from inline deletion change (envoyproxy#6988) Max heap fix (envoyproxy#7016) Add support to unregister from lifecycle notifications (envoyproxy#6984) build spdy_core_alt_svc_wire_format (envoyproxy#7010) ext_authz: Make sure initiateCall only called once (envoyproxy#6949) ... Signed-off-by: Michael Puncel <mpuncel@squareup.com>
Description: Return a handle from registration methods that can be used to unregister. This is needed for registrations made by objects that can have a shorter lifetime than the server (such as filters or access logs). When those objects are destroyed the callbacks will be invalidated so the registrations should be removed to avoid memory access violations.
Risk Level: low
Testing: unit tests
Signed-off-by: Elisha Ziskind eziskind@google.com