Fix #846 (#878)

* firstCommit * events is all wait for false flags * removed txt files Signed-off-by: tcormack <tcormack@mathworks.com> * serviceTracker tsan fixes Signed-off-by: tcormack <tcormack@mathworks.com> * deleted txt Signed-off-by: tcormack <tcormack@mathworks.com> * tsanSupppressions updated for various tests Signed-off-by: tcormack <tcormack@mathworks.com> * removed text files Signed-off-by: tcormack <tcormack@mathworks.com> * removed test * added comment Signed-off-by: tcormack <tcormack@mathworks.com> * updated suppresssions file with commentss * add sleep to stop spin * add unused flag to lock Signed-off-by: tcormack <tcormack@mathworks.com> * revert reformat * formatting * formatting * reformat * updating comments * update comment --------- Signed-off-by: tcormack <tcormack@mathworks.com>
CppMicroServices · Jul 12, 2023 · ab7d455 · ab7d455 · github-actions · Jul 12, 2023
1 parent fcc8022
commit ab7d455
Show file tree

Hide file tree

Showing 2 changed files with 45 additions and 7 deletions.
diff --git a/framework/test/bundles/libC1/TestBundleC1.cpp b/framework/test/bundles/libC1/TestBundleC1.cpp
@@ -85,7 +85,9 @@ namespace cppmicroservices
             tracker->Close();
 
             if (count != 0)
+            {
                 throw std::logic_error("Not unregistered all services");
+            }
         }
 
         void
@@ -104,7 +106,9 @@ namespace cppmicroservices
                 regs.Lock(), regs.v.emplace_back(std::move(reg));
                 count++;
                 if (i % 5 == 0)
+                {
                     regs.NotifyAll();
+                }
             }
         }
 
@@ -115,12 +119,6 @@ namespace cppmicroservices
             {
                 std::vector<ServiceRegistrationU> currRegs;
                 regs.Lock(), std::swap(currRegs, regs.v);
-                if (currRegs.empty())
-                {
-                    auto l = regs.Lock();
-                    regs.WaitFor(l, std::chrono::milliseconds(100), [this] { return !regs.v.empty(); });
-                    std::swap(currRegs, regs.v);
-                }
 
                 for (auto& reg : currRegs)
                 {
@@ -138,6 +136,13 @@ namespace cppmicroservices
                         count--;
                     }
                 }
+                /*
+                Rather than a condition variable, we are looping until regs is non-empty.
+                This sleep stops this function from looping too quickly and causing slow downs.
+                While this is less efficent than a condition variable, we are doing this in order to eliminate a
+                false-positive TSAN warning coming from the function wait_for.
+                */
+                std::this_thread::sleep_for(std::chrono::milliseconds(100));
             }
         }
 
@@ -164,7 +169,9 @@ namespace cppmicroservices
         ModifiedService(ServiceReferenceU const& reference, InterfaceMapConstPtr const& /*service*/)
         {
             if (reference.GetProperty("i") != 5)
+            {
                 throw std::logic_error("modified end match: wrong property");
+            }
             context.GetService(reference);
         }
 
@@ -173,7 +180,9 @@ namespace cppmicroservices
         {
             auto l = additionalReg.Lock();
             if (additionalReg.v)
+            {
                 additionalReg.v.Unregister();
+            }
         }
 
       private:
@@ -200,4 +209,4 @@ namespace cppmicroservices
 
 } // namespace cppmicroservices
 
-CPPMICROSERVICES_EXPORT_BUNDLE_ACTIVATOR(cppmicroservices::TestBundleC1Activator)
+CPPMICROSERVICES_EXPORT_BUNDLE_ACTIVATOR(cppmicroservices::TestBundleC1Activator)
diff --git a/tsan_suppressions.txt b/tsan_suppressions.txt
@@ -2,5 +2,34 @@
 race:cppmicroservices::ServiceListeners::RemoveServiceListener
 race:cppmicroservices::ServiceTracker*
 
+# BundleRegistry races and double lock false-positives
+# https://github.com/google/sanitizers/issues/1259
+# Calls to wait_for within BundleRegistry::Install, TSAN false-positive
+# Race condition on access of p.second.waitFlag within wait_for, TSAN false-positive
+mutex:TestSerialBundleInstall
+race:TestSerialBundleInstall
+
+# TestConfigurationAdminImpl race and double lock
+
+# CV and mutex used within test body, TSAN false-positive
+race:TestConfigurationAdminImpl_VerifyManagedServiceNotification_Test::TestBody
+mutex:TestConfigurationAdminImpl_VerifyManagedServiceNotification_Test::TestBody
+
+# CV and mutex used within test body, TSAN false-positive
+race:TestConfigurationAdminImpl_VerifyManagedServiceFactoryNotification_Test::TestBody
+mutex:TestConfigurationAdminImpl_VerifyManagedServiceFactoryNotification_Test::TestBody
+
+# CV and mutex used within test body, TSAN false-positive
+race:TestConfigurationAdminImpl_VerifyManagedServiceExceptionsAreLogged_Test::TestBody
+mutex:TestConfigurationAdminImpl_VerifyManagedServiceExceptionsAreLogged_Test::TestBody
+
+# Framework.LifeCycle
+mutex:cppmicroservices::FrameworkPrivate::Shutdown0
+mutex:Framework_LifeCycle_Test::TestBody
+
+# Assignment in FrameworkPrivate::WaitForStop and FrameworkPrivate::SystemShuttingdownDone_unlocked
+# to stopEvent, both protected, TSAN false-positive
+race:Framework_LifeCycle_Test::TestBody
+
 # Potential Deadlock in CCACtiveState
 deadlock:cppmicroservices::scrimpl::CCActiveState::Activate