Merge pull request accellera-official#104 from markfoodyburton/sc_pri…

…m_channel_thread_safety

Fix mutexes in sc_prim_channel

examples/sysc/async_suspend/collector.h

@@ -54,6 +54,7 @@ class collector
 
     void csvreport()
     {
+        std::lock_guard<std::mutex> guard(lock);
         cout << "event";
         for (auto kv : names)
         {

examples/sysc/async_suspend/node.h

@@ -36,6 +36,7 @@
 #include <mutex>
 #include <thread>
 #include <condition_variable>
+#include <atomic>
 
 #include "tlm.h"
 #include "tlm_utils/simple_initiator_socket.h"
@@ -110,8 +111,8 @@ SC_MODULE (asynctestnode)
     collector &col;                // just used to print out the results
 
 
-    bool running;
-    bool finished;
+    std::atomic<bool> running;
+    std::atomic<bool> finished;
 
     asynctestnode(sc_core::sc_module_name name, collector &c) :
         init_socket("output"),

src/sysc/communication/sc_prim_channel.cpp

@@ -150,8 +150,9 @@ class sc_prim_channel_registry::async_update_list
 {
 public:
 
-    bool pending() const
+    bool pending()
     {
+        sc_scoped_lock lock( m_mutex );
 	return m_push_queue.size() != 0;
     }
 
@@ -195,7 +196,6 @@ class sc_prim_channel_registry::async_update_list
 
     void attach_suspending( sc_prim_channel& p )
     {
-        sc_scoped_lock lock( m_mutex );
         std::vector<sc_prim_channel*>::iterator it =
           std::find(m_suspending_channels.begin(), m_suspending_channels.end(), &p);
         if ( it == m_suspending_channels.end() ) {
@@ -207,7 +207,6 @@ class sc_prim_channel_registry::async_update_list
 
     void detach_suspending( sc_prim_channel& p )
     {
-        sc_scoped_lock lock( m_mutex );
         std::vector<sc_prim_channel*>::iterator it =
           std::find(m_suspending_channels.begin(), m_suspending_channels.end(), &p);
         if ( it != m_suspending_channels.end() ) {

tests/CMakeLists.txt

@@ -128,8 +128,3 @@ if (CMAKE_CXX_COMPILER_ID STREQUAL "GNU")
   )
 endif()
 
-# Skip some test execution under some conditions
-if(APPLE OR HAS__aarch64__DEFINED)
-  # TODO: Test hangs frequently on macOS and/or ARM 64-bit setups
-  skip_test(systemc/kernel/sc_suspend)
-endif()

tests/systemc/kernel/sc_suspend/golden/sc_suspend.log

@@ -10,8 +10,6 @@ Info: tester: Suspend at SystemC time: 350 us
 
 Info: async_event: Called stage_callback at stage: SC_PRE_SUSPEND
 
-Info: child thread: Release child thread
-
 Info: tester: Restart SystemC time: 350 us
 
 Info: tester: Start to advance SystemC time: 350 us

tests/systemc/kernel/sc_suspend/sc_suspend.cpp

@@ -41,62 +41,85 @@
 #include <systemc>
 #include <thread>
 
-class async_event : public sc_core::sc_prim_channel, public sc_core::sc_event, public sc_core::sc_stage_callback_if
+class async_event : public sc_core::sc_prim_channel,
+                    public sc_core::sc_event,
+                    public sc_core::sc_stage_callback_if 
 {
 private:
-    int outstanding;
     sc_core::sc_time m_delay;
     std::thread::id tid;
     std::mutex mutex; // Belt and braces
+    bool outstanding=false;
 
 public:
-    async_event(bool start_attached = true): outstanding(0)
-    {
+    async_event(bool start_attached = true) {
         sc_core::sc_register_stage_callback(*this, sc_core::SC_PRE_PAUSE | sc_core::SC_PRE_SUSPEND | sc_core::SC_POST_SUSPEND);
         tid = std::this_thread::get_id();
-        outstanding = 0;
         enable_attach_suspending(start_attached);
     }
 
     void async_notify() { notify(); }
 
-    void notify(sc_core::sc_time delay = sc_core::sc_time(sc_core::SC_ZERO_TIME))
-    {
+    void notify() {
+        if (tid == std::this_thread::get_id()) {
+            sc_core::sc_event::notify();
+        } else {
+            notify(sc_core::SC_ZERO_TIME);
+        }
+    }
+    void notify(double d, sc_core::sc_time_unit u) {
+        notify(sc_core::sc_time(d, u));
+    }
+    void notify(sc_core::sc_time delay) {
         if (tid == std::this_thread::get_id()) {
             sc_core::sc_event::notify(delay);
         } else {
             mutex.lock();
             m_delay = delay;
-            outstanding++;
+            outstanding = true;
             mutex.unlock();
             async_request_update();
         }
     }
 
-    void async_attach_suspending() { this->sc_core::sc_prim_channel::async_attach_suspending(); }
+    bool triggered() const {
+        if (tid == std::this_thread::get_id()) {
+            return sc_core::sc_event::triggered();
+        } else {
+            SC_REPORT_ERROR("async_event",
+                            "It is an error to call triggered() from outside "
+                            "the SystemC thread");
+        }
+        return false;
+    }
+    void async_attach_suspending() {
+        this->sc_core::sc_prim_channel::async_attach_suspending();
+    }
 
-    void async_detach_suspending() { this->sc_core::sc_prim_channel::async_detach_suspending(); }
+    void async_detach_suspending() {
+        this->sc_core::sc_prim_channel::async_detach_suspending();
+    }
 
-    void enable_attach_suspending(bool e)
-    {
-        mutex.lock();
+    void enable_attach_suspending(bool e) {
         e ? this->async_attach_suspending() : this->async_detach_suspending();
-        mutex.unlock();
     }
 
 private:
-    void update(void)
-    {
+    void update(void) {
         mutex.lock();
         // we should be in SystemC thread
         if (outstanding) {
             sc_event::notify(m_delay);
-            outstanding--;
-            if (outstanding) request_update();
+            outstanding = false;
         }
         mutex.unlock();
     }
-
+    void start_of_simulation() {
+        // we should be in SystemC thread
+        if (outstanding) {
+            request_update();
+        }
+    }
     void stage_callback(const sc_core::sc_stage& stage)
     {
         std::ostringstream stage_str;
@@ -116,6 +139,7 @@ class async_event : public sc_core::sc_prim_channel, public sc_core::sc_event, p
 };
 
 SC_MODULE (tester) {
+    bool released = false;
     std::mutex mutex;
     std::condition_variable condition;
     std::thread t;
@@ -125,8 +149,7 @@ SC_MODULE (tester) {
     SC_CTOR (tester) :
         t([&] {
             std::unique_lock<decltype(mutex)> lock(mutex);
-            condition.wait(lock);
-            SC_REPORT_INFO("child thread", "Release child thread");
+            condition.wait(lock, [&](){return released;});
             start.notify();
         }),
         start(false)
@@ -157,6 +180,7 @@ SC_MODULE (tester) {
 
             // triggure the other thread to release us !
             std::lock_guard<decltype(mutex)> lock(mutex);
+            released = true;
             condition.notify_one();
         } else {
             SC_REPORT_INFO("tester", ("Unsuspend at SystemC time: " + sc_core::sc_time_stamp().to_string()).c_str());