diff --git a/gloop/perftools/tracing/sync_context.cc b/gloop/perftools/tracing/sync_context.cc index 92037e43..5610a910 100644 --- a/gloop/perftools/tracing/sync_context.cc +++ b/gloop/perftools/tracing/sync_context.cc @@ -170,17 +170,12 @@ SyncContext::Impl* SyncContext::Impl::RemoveListenerFromCurrent( auto [new_listener, success] = active_listener->Extract(listener); if (!PERFTOOLS_VERIFY(success)) return this; - // Signal end of sync session. This is the last event delivered - // where the listener is reachable through the active listener. + // Signal end of sync session and release listener. listener->OnTraceEndSync(active_sync_id_); - - // Set the new listener minus the extracted one and release the listener. - // We guarantee the removed listener can no longer be reached / invoked - // from a call the `ReleaseEventListener()`. - internal::set_active_event_listener(new_listener); listener->ReleaseEventListener(); - // Check if we have remaining listeners. If so, we are still "alive". + // Set the new listener and check if we have remaining listeners. + internal::set_active_event_listener(new_listener); if (new_listener != nullptr) return this; // All listeners are removed, delete ourselves. diff --git a/gloop/perftools/tracing/sync_context.h b/gloop/perftools/tracing/sync_context.h index 463463a0..a10ceeb3 100644 --- a/gloop/perftools/tracing/sync_context.h +++ b/gloop/perftools/tracing/sync_context.h @@ -251,11 +251,6 @@ class ABSL_ATTRIBUTE_TRIVIAL_ABI SyncContext { // This function is a no-op if `listener` is null. // If `listener` is not present in this instance, then this function will // check fail in debug builds, and release the listener in production builds. - // Any call to `ReleaseEventListener()` on the provided `listener` or any - // embedded (multiplexed) listeners inside `listeners` is guaranteed to - // occur _after_ the current active listener for the instance has been - // updated. I.e.: The listener is no longer invoked from traced calls from - // non trivial code executed by any listener specific release logic. void RemoveListenerFromCurrent(Access, TraceEventListener* listener); // Returns `true` if the provided listener is present in this instance. diff --git a/gloop/perftools/tracing/sync_context_test.cc b/gloop/perftools/tracing/sync_context_test.cc index 7989e5fc..577190a1 100644 --- a/gloop/perftools/tracing/sync_context_test.cc +++ b/gloop/perftools/tracing/sync_context_test.cc @@ -529,34 +529,15 @@ TEST_F(SyncContextTest, RemoveListenersFromCurrent) { EXPECT_CALL(mock1, OnTraceBeginSync(kMainSyncId, _)); EXPECT_CALL(mock2, OnTraceBeginSync(kMainSyncId, _)); WithContext with(std::move(root)); - TraceEventListener* active_listener = internal::active_event_listener(); - // EndSync() is the last sync event invoked. - EXPECT_CALL(mock2, OnTraceEndSync(kMainSyncId)).WillOnce([&](auto) { - EXPECT_THAT(internal::active_event_listener(), Eq(active_listener)); - }); - // Extract() is the final interaction where active listener is in place. - EXPECT_CALL(mock2, Extract(&mock2)).WillOnce([&](auto...) { - EXPECT_THAT(internal::active_event_listener(), Eq(active_listener)); - return std::pair(nullptr, true); - }); - // Release() is guaranteed to be invoked after the active listener is reset. - EXPECT_CALL(mock2, ReleaseEventListener()).WillOnce([&](auto...) { - EXPECT_THAT(internal::active_event_listener(), Eq(&mock1)); - }); + EXPECT_CALL(mock2, OnTraceEndSync(kMainSyncId)); + EXPECT_CALL(mock2, Extract(&mock2)); + EXPECT_CALL(mock2, ReleaseEventListener()); Context::RemoveListenerFromCurrent(&mock2); - // EndSync() is the last sync event invoked. EXPECT_CALL(mock1, OnTraceEndSync(kMainSyncId)); - // Extract() is the final interaction where active listener is in place. - EXPECT_CALL(mock1, Extract(&mock1)).WillOnce([&](auto...) { - EXPECT_THAT(internal::active_event_listener(), Eq(&mock1)); - return std::pair(nullptr, true); - }); - // Release() is guaranteed to be invoked after the active listener is reset. - EXPECT_CALL(mock1, ReleaseEventListener()).WillOnce([&](auto...) { - EXPECT_THAT(internal::active_event_listener(), Eq(nullptr)); - }); + EXPECT_CALL(mock1, Extract(&mock1)); + EXPECT_CALL(mock1, ReleaseEventListener()); Context::RemoveListenerFromCurrent(&mock1); }