From 5386fe6fbc577165e6fd597e2cdeeca2c84be6a3 Mon Sep 17 00:00:00 2001
From: Ichthyostega <prg@ichthyostega.de>
Date: Sun, 7 Feb 2010 17:31:28 +0100
Subject: [PATCH] clarify (and treat) the race; considered to be test-only

After removing the explicit locking, there is a small
race in case of a "floundering" (simulated) subsystem:
the starting context may go away before the child thread
actually teminates. I consider this a shortcoming of
this test fixture, which isn't intended to be an example
of a real world subsystem, but rather focusses on
error detection within the subsystem runner.
---
 tests/lib/subsystem-runner-test.cpp | 28 +++++++++++++++++++++-------
 1 file changed, 21 insertions(+), 7 deletions(-)

diff --git a/tests/lib/subsystem-runner-test.cpp b/tests/lib/subsystem-runner-test.cpp
index e28a5b368..8e1ecb0be 100644
--- a/tests/lib/subsystem-runner-test.cpp
+++ b/tests/lib/subsystem-runner-test.cpp
@@ -61,6 +61,13 @@ namespace test  {
        *  shutdown request every XX milliseconds */
       const uint TICK_DURATION_ms = 5;
       
+      /** due to a shortcoming of this test fixture,
+       *  a floundering subsystem continues to run for
+       *  a short time after the sync barrier.
+       *  Relevant for singleSubsys_start_failure().
+       */
+      const uint DELAY_FOR_FLOUNDERING_THRAD_ms = 20;
+      
       /** dummy options just to be ignored */
       util::Cmdline dummyArgs ("");
       lumiera::Option dummyOpt (dummyArgs);
@@ -75,12 +82,9 @@ namespace test  {
       
       /** 
        * A simulated "Lumiera Subsystem".
-       * It is capable of starting a separate thread,
-       * which may terminate regularly after a random
-       * time, or may fail in various ways. 
-       * The behaviour is controlled by a number of
-       * definitions, given at construction in
-       * logic predicate notation.
+       * It is capable of starting a separate thread, which may terminate regularly
+       * after a random time, or may fail in various ways. The behaviour is controlled
+       * by a number of definitions, given at construction in logic predicate notation.
        */
       class MockSys
         : public lumiera::Subsys
@@ -262,7 +266,7 @@ namespace test  {
         run (Arg)
           {
             singleSubsys_complete_cycle();
-            singleSubsys_start_failure();      ////////////////////////////TODO: Valgrind logs an invalid read from here here, at line 172
+            singleSubsys_start_failure();
             singleSubsys_emegency_exit();
             
             dependentSubsys_complete_cycle();
@@ -289,6 +293,15 @@ namespace test  {
           }
         
         
+        /** @note as this test focuses on the SubsystemRunner, the mock subsystem
+         *  is implemented rather simplistic. Especially, there is a race when a
+         *  subsystem is configured to "fail" -- because in this case the starting
+         *  context may go away before the remainder of the subsystem thread has
+         *  executed after the sync() barrier. Especially in this case, no MockSys
+         *  actually starts without failure, and thus the SubsystemRunner::wait()
+         *  has no guarding effect. This can be considered a shortcoming of the
+         *  test fixture; a well behaved subsystem won't just go away...
+         */
         void
         singleSubsys_start_failure()
           {
@@ -306,6 +319,7 @@ namespace test  {
             VERIFY_ERROR (LOGIC, runner.maybeRun (unit3) );     // incorrect behaviour trapped
             VERIFY_ERROR (LOGIC, runner.maybeRun (unit4) );     // detected that the subsystem didn't come up
             
+            usleep (DELAY_FOR_FLOUNDERING_THRAD_ms * 1000);     // preempt to allow unit4 to go away
             runner.wait();
             
             ASSERT (!unit1.isRunning());