Fix #710 ServiceTracker TSAN warnings (#883)

* fixed tracker race * fixing tsanSupp * fixing tsan mistakes * fixed tracker race * tsan_suppresssions mistype * fixed comments * typos
CppMicroServices · Jul 12, 2023 · dc9142a · dc9142a · github-actions · Jul 12, 2023
1 parent ab7d455
commit dc9142a
Show file tree

Hide file tree

Showing 2 changed files with 19 additions and 4 deletions.
diff --git a/framework/include/cppmicroservices/detail/ServiceTracker.hpp b/framework/include/cppmicroservices/detail/ServiceTracker.hpp
@@ -151,9 +151,21 @@ namespace cppmicroservices
     void
     ServiceTracker<S, T>::Close()
     {
+
+        /*
+        The call to RemoveListener() below must be done while the ServiceTracker object is unlocked because of a
+        possibility for reentry from customer code. Therefore, we have to swap d->listenerToken to a local variable and
+        replace it with a default constructed ListenerToken object.
+        */
+        ListenerToken swappedToken;
+        {
+            auto l = d->Lock();
+            US_UNUSED(l);
+            std::swap(d->listenerToken, swappedToken);
+        }
         try
         {
-            d->context.RemoveListener(std::move(d->listenerToken));
+            d->context.RemoveListener(std::move(swappedToken));
         }
         catch (std::runtime_error const& /*e*/)
         {

diff --git a/tsan_suppressions.txt b/tsan_suppressions.txt
@@ -1,6 +1,9 @@
 # ServiceTracker/ServiceListener races
-race:cppmicroservices::ServiceListeners::RemoveServiceListener
-race:cppmicroservices::ServiceTracker*
+# Call to WaitFor() in WaitForService() leads to TSAN false-positive
+mutex:ServiceTrackerTestFixture_TestServiceTracker_Test::TestBody
+
+# expected race condition that IsRemoved() checks for
+race:ServiceTrackerTestFixture_DefaultCustomizerServiceTrackerCloseRace_Test::TestBody
 
 # BundleRegistry races and double lock false-positives
 # https://github.com/google/sanitizers/issues/1259
@@ -32,4 +35,4 @@ mutex:Framework_LifeCycle_Test::TestBody
 race:Framework_LifeCycle_Test::TestBody
 
 # Potential Deadlock in CCACtiveState
-deadlock:cppmicroservices::scrimpl::CCActiveState::Activate
+deadlock:cppmicroservices::scrimpl::CCActiveState::Activate