Update shared_mutex thirdparty to don't prioritize writers (#3091)

* Update shared_mutex thirdparty to not prioritize writers (#2976) * Add test for multithreaded creation of readers on a single subscriber. Signed-off-by: Miguel Company <MiguelCompany@eprosima.com> * Add DataWriter. Signed-off-by: Miguel Company <MiguelCompany@eprosima.com> * Refs 15766: Refactor of shared_mutex to select writer priority Signed-off-by: Miguel Barro <miguelbarro@eprosima.com> * Refs 15766: CMake update to force third party shared mutex if the framework prioritizes writing. Signed-off-by: Miguel Barro <miguelbarro@eprosima.com> * Refs 15766: Add atomic support for some debian distros Signed-off-by: Miguel Barro <miguelbarro@eprosima.com> * Refs 15766: Allow recursiveness on participant endpoint collection mutexes. Now they always allow them. Signed-off-by: Miguel Barro <miguelbarro@eprosima.com> * Refs 15766: linter pass Signed-off-by: Miguel Barro <miguelbarro@eprosima.com> * Refs 15766: Generate the config.h file when the USE_THIRDPARTY_SHARED_MUTEX value is already specified. Signed-off-by: Miguel Barro <miguelbarro@eprosima.com> * Refs 15766: Mandatory piggyback: avoid polution on free_pools_ collection Signed-off-by: Miguel Barro <miguelbarro@eprosima.com> * Refs 15766: Add some missing members to shared_lock thirdparty Signed-off-by: Miguel Barro <miguelbarro@eprosima.com> * Refs 15766: linter Signed-off-by: Miguel Barro <miguelbarro@eprosima.com> * Refs 15766. shared_mutex testing Signed-off-by: Miguel Barro <miguelbarro@eprosima.com> * Refs 15766. fixing gtest backward compatibility issues Signed-off-by: Miguel Barro <miguelbarro@eprosima.com> * Refs 15766. Add a new test to check priority is right Signed-off-by: Miguel Barro <miguelbarro@eprosima.com> * Refs 15766. Fixing gcc build warnings Signed-off-by: Miguel Barro <miguelbarro@eprosima.com> * Refs 15766. Fixing clang build warnings Signed-off-by: Miguel Barro <miguelbarro@eprosima.com> * Refs 15766. Make thirdparty versions always available even if none is used as eprosima::shared_mutex. Signed-off-by: Miguel Barro <miguelbarro@eprosima.com> * Refs 15766. Addressing reviewers comments Signed-off-by: Miguel Barro <miguelbarro@eprosima.com> * Refs 15766. Fixing sync issue on ProxyPool. Signed-off-by: Miguel Barro <miguelbarro@eprosima.com> Signed-off-by: Miguel Company <MiguelCompany@eprosima.com> Signed-off-by: Miguel Barro <miguelbarro@eprosima.com> Co-authored-by: Miguel Company <MiguelCompany@eprosima.com> * Fixing gcc warnings Signed-off-by: Miguel Barro <miguelbarro@eprosima.com> * make sure config.h is always included through fastrtps_dll.h as is expected. Do we need a foreport? Signed-off-by: Miguel Barro <miguelbarro@eprosima.com> Signed-off-by: Miguel Company <MiguelCompany@eprosima.com> Signed-off-by: Miguel Barro <miguelbarro@eprosima.com> Co-authored-by: MiguelBarro <45819833+MiguelBarro@users.noreply.github.com> Co-authored-by: Miguel Company <MiguelCompany@eprosima.com> Co-authored-by: Miguel Barro <miguelbarro@eprosima.com>
eProsima · Dec 1, 2022 · f3b6fa8 · f3b6fa8
1 parent 6845e62
commit f3b6fa8
Show file tree

Hide file tree

Showing 14 changed files with 857 additions and 92 deletions.
diff --git a/cmake/common/gtest.cmake b/cmake/common/gtest.cmake
@@ -59,6 +59,11 @@ macro(add_gtest)
             # Normal tests
             file(STRINGS ${GTEST_SOURCE_FILE} GTEST_TEST_NAMES REGEX "^([T][Y][P][E][D][_])?TEST")
             foreach(GTEST_TEST_NAME ${GTEST_TEST_NAMES})
+
+                if(GTEST_TEST_NAME MATCHES "TYPED_TEST_SUITE")
+                    continue()
+                endif()
+
                 string(REGEX REPLACE ["\) \(,"] ";" GTEST_TEST_NAME ${GTEST_TEST_NAME})
                 list(GET GTEST_TEST_NAME 1 GTEST_GROUP_NAME)
                 list(GET GTEST_TEST_NAME 3 GTEST_TEST_NAME)

diff --git a/cmake/modules/check_shared_mutex_priority.cpp b/cmake/modules/check_shared_mutex_priority.cpp
@@ -0,0 +1,69 @@
+// Copyright 2022 Proyectos y Sistemas de Mantenimiento SL (eProsima).
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+/**
+ * @file check_shared_mutex_priority.cpp
+ *
+ */
+
+#include <atomic>
+#include <condition_variable>
+#include <iostream>
+#include <mutex>
+#include <shared_mutex>
+#include <thread>
+
+using namespace std;
+
+int main()
+{
+    shared_mutex sm;
+    atomic_bool mark = false;
+
+    // take first shared lock
+    sm.lock_shared();
+
+    // signal is taken
+    thread exclusive([&]()
+            {
+                mark = true;
+                lock_guard<shared_mutex> guard(sm);
+            });
+
+    // Wait till the thread takes the lock
+    do
+    {
+        this_thread::sleep_for(chrono::milliseconds(100));
+    }
+    while (!mark);
+
+    // try take the second shared lock
+    bool success = sm.try_lock_shared();
+    if (success)
+    {
+        sm.unlock_shared();
+        cout << "PTHREAD_RWLOCK_PREFER_READER_NP" << endl;
+    }
+    else
+    {
+        cout << "PTHREAD_RWLOCK_PREFER_WRITER_NONRECURSIVE_NP" << endl;
+    }
+
+    // release first lock
+    sm.unlock_shared();
+    // wait for the main thread
+    exclusive.join();
+
+    return 0;
+}
diff --git a/include/fastrtps/config.h.in b/include/fastrtps/config.h.in
@@ -78,6 +78,8 @@
 #define HAVE_SQLITE3 @HAVE_SQLITE3@
 #endif /* ifndef HAVE_SQLITE3 */
 
+// Using thirdparty shared_mutex
+#cmakedefine01 USE_THIRDPARTY_SHARED_MUTEX
 
 // TLS support
 #ifndef TLS_FOUND

diff --git a/include/fastrtps/utils/ProxyPool.hpp b/include/fastrtps/utils/ProxyPool.hpp
@@ -154,6 +154,9 @@ class ProxyPool
 
         // return the resource
         mask_.set(idx);
+
+        // notify the resource is free
+        cv_.notify_one();
     }
 
 public:

diff --git a/include/fastrtps/utils/TimedConditionVariable.hpp b/include/fastrtps/utils/TimedConditionVariable.hpp
@@ -18,7 +18,7 @@
 
 #ifndef _UTILS_TIMEDCONDITIONVARIABLE_HPP_
 #define _UTILS_TIMEDCONDITIONVARIABLE_HPP_
-#include <fastrtps/config.h>
+#include <fastrtps/fastrtps_dll.h>
 
 /*
    NOTE: Windows implementation temporary disabled due to aleatory high CPU consumption when

diff --git a/include/fastrtps/utils/collections/foonathan_memory_helpers.hpp b/include/fastrtps/utils/collections/foonathan_memory_helpers.hpp
@@ -24,7 +24,7 @@
 #include <foonathan/memory/detail/debug_helpers.hpp>
 
 #include "ResourceLimitedContainerConfig.hpp"
-#include "fastrtps/config.h"
+#include <fastrtps/fastrtps_dll.h>
 
 namespace eprosima {
 namespace fastrtps {