Merge pull request #38 from Flow-IPC/tsan_fun

TSAN caught subtle error when exiting transport_test (exercise mode); worker loop should be stopped before things that tasks posted-onto it touch get destroyed in dtor. So doing that now. Assuming I got that right transport_test will finish fine with TSAN, in exercise mode, at least in heap sub-mode (SHM-classic and SHM-jemalloc sub-modes TBD). Also updating submodules (comment/spacing only changes).

.github/workflows/main.yml

@@ -231,7 +231,6 @@ jobs:
               Shm_pool_collection_test.Interface:\
               Shm_pool_collection_test.Multiprocess:\
               Shm_session_data_test.Multithread_object_database
-            skip-transport-tests-ex: true # TODO: Explain why briefly (even though there's prob a ticket).
             sanitizer-name: tsan
             no-lto: false # See the other *SAN; but for now I heed jkontrik's words to keep consistent with non-*SAN.
           - id: relwithdebinfo-msan
@@ -679,7 +678,7 @@ jobs:
     # This follows the instructions in bin/transport_test/README.txt.
     - name: Prepare run script for [transport_test - Scripted mode] variations below
       if: |
-        (!cancelled()) && (!matrix.build-test-cfg.skip-transport-tests-sc)
+        !cancelled()
       run: |
         cat <<'EOF' > ${{ env.install-dir }}/bin/run_transport_test_sc.sh
         echo "Log level: [$1]."
@@ -708,7 +707,7 @@ jobs:
     - name: Run integration test [transport_test - Scripted mode]
       id: transport_test_scripted
       if: |
-        (!cancelled()) && (!matrix.build-test-cfg.skip-transport-tests-sc)
+        !cancelled()
       run: |
         /usr/bin/bash -e \
           ${{ env.install-dir }}/bin/run_transport_test_sc.sh \
@@ -729,7 +728,7 @@ jobs:
 
     - name: Prepare IPC-session safety-friendly run-time environment for [transport_test - Exercise mode]
       if: |
-        (!cancelled()) && (!matrix.build-test-cfg.skip-transport-tests-ex)
+        !cancelled()
       run: |
         mkdir -p ~/bin/ex_srv_run ~/bin/ex_cli_run
         mkdir -p /tmp/var/run
@@ -739,7 +738,7 @@ jobs:
 
     - name: Prepare run script for [transport_test - Exercise mode] variations below
       if: |
-        (!cancelled()) && (!matrix.build-test-cfg.skip-transport-tests-ex)
+        !cancelled()
       run: |
         cat <<'EOF' > ${{ env.install-dir }}/bin/run_transport_test_ex.sh
         # Script created by pipeline during job.
@@ -774,7 +773,7 @@ jobs:
     - name: Run integration test [transport_test - Exercise mode - Heap sub-mode]
       id: transport_test_ex_heap
       if: |
-        (!cancelled()) && (!matrix.build-test-cfg.skip-transport-tests-ex)
+        !cancelled()
       run: |
         /usr/bin/bash -e \
           ${{ env.install-dir }}/bin/run_transport_test_ex.sh \
@@ -790,8 +789,8 @@ jobs:
 
     - name: Run integration test [transport_test - Exercise mode - SHM-classic sub-mode]
       id: transport_test_ex_shm_c
-      if: |
-        (!cancelled()) && (!matrix.build-test-cfg.skip-transport-tests-ex)
+      if: | # TODO: Skipping TSAN mode for this; currently unstable; working on it (ticket).
+        (!cancelled()) && (matrix.build-test-cfg.sanitizer-name != 'tsan')
       run: |
         /usr/bin/bash -e \
           ${{ env.install-dir }}/bin/run_transport_test_ex.sh \
@@ -807,8 +806,8 @@ jobs:
 
     - name: Run integration test [transport_test - Exercise mode - SHM-jemalloc sub-mode]
       id: transport_test_ex_shm_j
-      if: |
-        (!cancelled()) && (!matrix.build-test-cfg.skip-transport-tests-ex)
+      if: | # TODO: Skipping TSAN mode for this; currently unstable; working on it (ticket).
+        (!cancelled()) && (matrix.build-test-cfg.sanitizer-name != 'tsan')
       run: |
         /usr/bin/bash -e \
           ${{ env.install-dir }}/bin/run_transport_test_ex.sh \

test/suite/transport_test/ex_cli.hpp

@@ -32,6 +32,7 @@ class Ex_cli : public Ex_guy
   using Session = Session_t;
 
   Ex_cli(flow::log::Logger* logger_ptr, flow::log::Logger* ipc_logger_ptr);
+  ~Ex_cli();
 
   bool run();
 
@@ -151,6 +152,13 @@ CLASS::Ex_cli(flow::log::Logger* logger_ptr, flow::log::Logger* ipc_logger_ptr)
   // OK.
 }
 
+TEMPLATE
+CLASS::~Ex_cli()
+{
+  // See Ex_srv dtor.  Same reasoning here.
+  Base::stop_worker();
+}
+
 TEMPLATE
 bool CLASS::run()
 {

test/suite/transport_test/ex_guy.cpp

@@ -72,6 +72,11 @@ void Ex_guy::done_and_done(bool ok)
   m_exit_promise.set_value(ok);
 }
 
+void Ex_guy::stop_worker()
+{
+  m_loop.stop();
+}
+
 flow::util::Task_engine* Ex_guy::task_engine()
 {
   return m_loop.task_engine().get();

test/suite/transport_test/ex_guy.hpp

@@ -67,6 +67,7 @@ class Ex_guy : public flow::log::Log_context
   static Sptr<T> make_sptr(Args&&... args);
 
   void done_and_done(bool ok);
+  void stop_worker();
 
   template<typename Task>
   bool run(bool srv_else_cli, Task&& body);

test/suite/transport_test/ex_srv.hpp

@@ -36,6 +36,7 @@ class Ex_srv : public Ex_guy
   using Server = Server_t;
 
   Ex_srv(flow::log::Logger* logger_ptr, flow::log::Logger* ipc_logger_ptr);
+  ~Ex_srv();
 
   bool run();
 
@@ -171,6 +172,18 @@ CLASS::Ex_srv(flow::log::Logger* logger_ptr, flow::log::Logger* ipc_logger_ptr)
   // Yeah.
 }
 
+TEMPLATE
+CLASS::~Ex_srv()
+{
+  /* A bit subtle but: Our base Ex_cli has the Single_thread_task_loop (thread W), where things are post()ed
+   * (directly or via async_wait() at least) to execute; those things will touch parts of *this.  *this is
+   * about to be destroyed; so there is a race wherein last-second stuff can try to touch parts that are
+   * being destroyed at the same time (data race, caught by TSAN, yay!), or even after they are destroyed
+   * (use-after-free, which ASAN would've caught).  At any rate this will synchronously stop thread W before
+   * such things can happen. */
+  Base::stop_worker();
+}
+
 TEMPLATE
 bool CLASS::run()
 {