Session-Subsytem(#318): finish review of locking and lifecycle sanity

This subsystem as such can be considered as implemented now,
while several details still wait to be filled in.

src/proc/control/proc-dispatcher.cpp

@@ -338,7 +338,7 @@ namespace control {
       new DispatcherLoop (
             [=] (string* problemMessage)
                 {
-                  runningLoop_.reset();   //////////////////////TODO: Race in ProcDispatcher
+                  ProcDispatcher::endRunningLoopState();
                   termNotification(problemMessage);
                 }));
     
@@ -361,17 +361,41 @@ namespace control {
   
   
   /** signal to the loop thread that it needs to terminate.
-   * @warning dangerous operation; must not block nor throw
+   * @note the immediate consequence is to close SessionCommandService
-   * 
-   * @todo need to re-check the logic, once the loop is fully implemented; ensure there is nothing on this call path that can block or throw!!! 
    */
   void
   ProcDispatcher::requestStop()  noexcept
   {
+    try {
       Lock sync(this);
       if (runningLoop_)
         runningLoop_->requestStop();
     }
+    ERROR_LOG_AND_IGNORE (command, "Request for Session Loop Thread to terminate");
+  }
+  
+  
+  /** @internal clean-up when leaving the session loop thread.
+   *  This function is hooked up in to the termination callback,
+   *  and is in fact the only one to delete the loop PImpl. We
+   *  take the (outer) lock on ProcDispatcher to ensure no one
+   *  commits anything to the DispatcherLoop object while being
+   *  deleted. The call itself, while technically originating
+   *  from within DispatcherLoop::runSessionThread(), relies
+   *  solely on stack based context data and is a tail call.
+   */
+  void
+  ProcDispatcher::endRunningLoopState()
+  {
+    Lock sync(this);
+    if (runningLoop_)
+      runningLoop_.reset();  // delete DispatcherLoop object
+    else
+      WARN (command, "clean-up of DispatcherLoop invoked, "
+                     "while ProcDispatcher is not marked as 'running'. "
+                     "Likely an error in lifecycle logic, as the only one "
+                     "intended to delete this object is the loop thread itself.");
+  }
   
   
   /** activate processing of enqueued session commands.
@@ -417,6 +441,7 @@ namespace control {
   }
   
   
+  /** discard any commands waiting in the dispatcher queue */
   void
   ProcDispatcher::clear()
   {

src/proc/control/proc-dispatcher.hpp

@@ -102,6 +102,8 @@ namespace control {
       
       bool empty()  const ;
       
+    private:
+      void endRunningLoopState();
     };
   
   

src/proc/facade.cpp

@@ -21,14 +21,35 @@
 * *****************************************************/
 
 
+/** @file facade.cpp
+ ** Implementation of subsystem lifecycle behaviour for the core parts of Proc-Layer.
+ ** - The »session subsystem« is responsible for accepting operations to work on the
+ **   session datastructure, and it will trigger the Builder to reflect those changes
+ **   into a suitable render nodes network
+ ** - The »play out subsystem« is able to _perform_ such a render nodes network
+ **   for video playback and rendering.
+ ** 
+ ** lumiera::Subsys and lumiera::SubsystemRunner together define a protocol for some
+ ** large scale building blocks of the whole application to be started and terminated.
+ ** Typically this entails to create a dedicated thread to manage the top level concerns
+ ** for the given subsystem, and to create registration and services for public access:
+ ** - to operate on the session, use the proc::control::SessionCommand facade
+ ** - playback and render operations can be operated by the lumiera::Play facade
+ ** 
+ ** @see ProcDispatcher
+ ** @see OutputDirector
+ ** @see subsys.hpp
+ ** @see main.cpp
+ **
+ */
+
+
 #include "proc/facade.hpp"
 #include "lib/depend.hpp"
-//#include "lib/sync.hpp"
 #include "proc/control/proc-dispatcher.hpp"
 #include "proc/play/output-director.hpp"
 
 #include <string>
-//#include <memory>
 
 
 namespace proc {
@@ -37,8 +58,6 @@ namespace proc {
   using std::unique_ptr;
   using lumiera::Subsys;
   using lumiera::Option;
-  using lib::Sync;
-//  using lib::RecursiveLock_NoWait;
   using proc::control::ProcDispatcher;
   
   

src/proc/facade.hpp

@@ -22,11 +22,12 @@
 
 /** @file facade.hpp
  ** Top level entrance point and facade for the Proc-Layer.
- ** The middle layer of the application holds a session with the
+ ** The middle layer of the application holds a session with the high-level model,
- ** high-level model, to be translated by the Builder into a node network,
+ ** to be translated by the Builder into a node network, which can be _performed_
- ** which can be \em performed by the Engine to render output.
+ ** by the Engine to render output.
  ** 
  ** @see common.hpp
+ ** @see main.cpp
  ** 
  */
 
@@ -49,8 +50,12 @@ namespace proc {
    * @todo this is a dummy placeholder as of 1/2009. Currently, there
    *       is only implementation-level code within the Proc-Layer and
    *       the interfaces need to be worked out.
-   * @note at least the Play/Output subsystem slowly turns into
+   * @todo at least the Play/Output subsystem slowly turns into
    *       something real, as of 6/2011
+   * @todo and as of 1/2017, the Session subsystem is basically
+   *       implemented and it became clear that there won't be
+   *       a "Builder subsystem". So this interface might now
+   *       be considered roughly complete...
    * 
    */
   struct Facade

wiki/renderengine.html

@@ -6410,17 +6410,23 @@ And last but not least: the difficult part of this whole concept is encapsulated
 
 {{red{WIP ... draft}}}</pre>
 </div>
-<div title="SessionSubsystem" creator="Ichthyostega" modifier="Ichthyostega" created="201612150347" modified="201612150453" tags="def impl SessionLogic img" changecount="9">
+<div title="SessionSubsystem" creator="Ichthyostega" modifier="Ichthyostega" created="201612150347" modified="201701050238" tags="def impl SessionLogic img" changecount="10">
 <pre>//A subsystem within Proc-Layer, responsible for lifecycle and access to the editing [[Session]].//
 [img[Structure of the Session Subsystem|uml/Session-subsystem.png]]
 
 !Structure
-The ProcDispatcher is at the heart of the //Session Subsystem.// Because the official interface for working on the session, the {{{SessionCommand}}} façade, is expressed in terms of sending command messages to invoke predefined [[commands|CommandHandling]] to operate on the SessionInterface, the actual implementation of such a {{{SessionCommandService}}} needs a component actually to enqueue and dispatch those commands -- which is the {{{DispatcherLoop}}} within the ProcDispatcher. As usual, the ''lifecycle'' is controlled by a subsystem descriptor, which starts and stopps the whole subsystem; and this starting and stopping in turn translates into starting/stopping of the dispatcher loop. On the other hand, //activation of the dispatcher,// which means actively to dispatch commands, is controlled by the lifecycle of the session proper. The latter is just a data structure, and can be loaded / saved and rebuilt through the ''session manager''.
+The ProcDispatcher is at the heart of the //Session Subsystem.// Because the official interface for working on the session, the {{{SessionCommand}}} façade, is expressed in terms of sending command messages to invoke predefined [[commands|CommandHandling]] to operate on the SessionInterface, the actual implementation of such a {{{SessionCommandService}}} needs a component actually to enqueue and dispatch those commands -- which is the {{{DispatcherLoop}}} within the ProcDispatcher. As usual, the ''lifecycle'' is controlled by a subsystem descriptor, which starts and stops the whole subsystem; and this starting and stopping in turn translates into starting/stopping of the dispatcher loop. On the other hand, //activation of the dispatcher,// which means actively to dispatch commands, is controlled by the lifecycle of the session proper. The latter is just a data structure, and can be loaded / saved and rebuilt through the ''session manager''.
 
 !Lifecycle
+As far as lifecycle is concerned, the »session subsystem« has to be distinguished from the //session proper,// which is just a data structure with its own, separate lifecycle considerations. Accessing the session data only makes sense when this data structure is fully loaded, while the //session subsystem,// deals with performing commands on the session and with triggering the builder runs.
+
+!!!start-up
+The session subsystem lifecycle translates into method invocations on the {{{ProcDispatcher}}}, which in turn manages the parts actually implementing the session command processing and builder operations. This relation is expressed by holding onto the implementation as a PImpl. As long as the {{{DispatcherLoop}}} object exists, the session subsystem can be considered in //running state.// This is equivalent to the following
+* the ''session loop thread'' is spawned. This thread performs all of the session and builder operations (single-threaded).
+* the {{{SessionCommandService}}} is started and connected as implementation of the {{{SessionCommand}}} façade.
 
 !!!shutdown
-Shutdown is initiated by sending a message to the dispatcher loop. {{red{TODO 12/2016}}} need to ensure the loop terminates reliably then! When loop terminates, the {{{sigTerm}}} of the SessionSubsystem is invoked, which then in turn causes the DispatcherLoop thread to be reaped.
+Shutdown is initiated by sending a message to the dispatcher loop. This causes the internal loop control to wake up and leave the loop, possibly after finishing a command or builder run currently in progress. When leaving the loop, the {{{sigTerm}}} of the SessionSubsystem is invoked, which then in turn causes the {{{DispatcherLoop}}} object to be deleted and the ProcDispatcher thus returned into halted state. 
 </pre>
 </div>
 <div title="SideBarOptions" modifier="CehTeh" created="200706200048">

wiki/thinkPad.ichthyo.mm

@@ -503,14 +503,26 @@
 <node CREATED="1483477519974" ID="ID_1731425552" MODIFIED="1483477535864" TEXT="Term-Signal k&#xf6;nnte blocken">
 <icon BUILTIN="help"/>
 </node>
-<node BACKGROUND_COLOR="#eee5c3" COLOR="#990000" CREATED="1483484052785" ID="ID_667055412" MODIFIED="1483484091053" TEXT="Race beim runningLoop_.reset()">
+<node COLOR="#338800" CREATED="1483484052785" ID="ID_667055412" MODIFIED="1483581561638" TEXT="Race beim runningLoop_.reset()">
-<icon BUILTIN="flag-yellow"/>
+<icon BUILTIN="button_ok"/>
 </node>
-<node CREATED="1483490316758" ID="ID_1734989824" MODIFIED="1483490376306" TEXT="nur das Term-Signal kann DispatcherLoop deleten">
+<node COLOR="#338800" CREATED="1483490459499" ID="ID_1006586365" MODIFIED="1483581568462" TEXT="Thread will sich selbst reapen ==&gt; Deadlock">
+<icon BUILTIN="button_ok"/>
+</node>
+<node CREATED="1483581573147" FOLDED="true" ID="ID_989149672" MODIFIED="1483581738496" TEXT="durch Umbau gel&#xf6;st">
+<icon BUILTIN="info"/>
+<node CREATED="1483490316758" ID="ID_1734989824" MODIFIED="1483581603244" TEXT="nur das Term-Signal kann DispatcherLoop deleten">
+<icon BUILTIN="idea"/>
+</node>
+<node CREATED="1483581605968" ID="ID_227347532" MODIFIED="1483581655738" TEXT="mu&#xdf; das &#xe4;u&#xdf;ere Lock auf ProcDispatcher verwenden"/>
+<node CREATED="1483581657849" ID="ID_1078225675" MODIFIED="1483581670643" TEXT="damit sind konkurrierende Zugriffe auf die Loop geblockt"/>
+<node CREATED="1483581671183" ID="ID_186079754" MODIFIED="1483581680474" TEXT="und danach ist der PImpl bereits auf NULL gesetzt"/>
+<node CREATED="1483581583411" ID="ID_650761020" MODIFIED="1483581594997" TEXT="thread join ist &#xfc;berfl&#xfc;ssig"/>
+<node CREATED="1483581640739" ID="ID_560356092" MODIFIED="1483581652445" TEXT="das Objekt wird in einem tail-call gel&#xf6;scht"/>
+<node CREATED="1483581682701" ID="ID_747399934" MODIFIED="1483581695879" TEXT="dieser callback selber vewrwendet nur Werte vom Stack"/>
+<node CREATED="1483581696443" ID="ID_373783285" MODIFIED="1483581733454" TEXT="daher kann das Objekt gefahrlos &quot;sich selber l&#xf6;schen&quot;">
 <icon BUILTIN="idea"/>
 </node>
-<node BACKGROUND_COLOR="#fdfdcf" COLOR="#ff0000" CREATED="1483490459499" ID="ID_1006586365" MODIFIED="1483490478840" TEXT="Thread will sich selbst reapen ==&gt; Deadlock">
-<icon BUILTIN="flag-pink"/>
 </node>
 </node>
 <node CREATED="1482464502542" ID="ID_284397306" MODIFIED="1482464609118" TEXT="Vorsicht beim Schlie&#xdf;en der Session">
@@ -518,12 +530,13 @@
 <icon BUILTIN="messagebox_warning"/>
 </node>
 </node>
-<node CREATED="1481599430804" ID="ID_1105415989" MODIFIED="1481599438887" TEXT="Forwarding-Operationen implementieren">
+<node CREATED="1481599430804" HGAP="19" ID="ID_1105415989" MODIFIED="1483581921613" TEXT="Forwarding-Operationen implementieren" VSHIFT="10">
+<icon BUILTIN="button_ok"/>
 <node CREATED="1481686902156" ID="ID_1526215617" MODIFIED="1481686910633" TEXT="aktivieren /deaktivieren">
 <icon BUILTIN="button_ok"/>
 </node>
-<node BACKGROUND_COLOR="#eee5c3" COLOR="#990000" CREATED="1481686911938" ID="ID_17287395" MODIFIED="1481686919619" TEXT="stoppen">
+<node CREATED="1481686911938" ID="ID_17287395" MODIFIED="1483581788279" TEXT="stoppen">
-<icon BUILTIN="flag-yellow"/>
+<icon BUILTIN="button_ok"/>
 </node>
 </node>
 <node CREATED="1481599594399" ID="ID_1017459823" MODIFIED="1481687087609" TEXT="entscheiden, wer die Session &#xf6;ffnet">
@@ -567,16 +580,27 @@
 <icon BUILTIN="yes"/>
 </node>
 </node>
-</node>
+<node COLOR="#338800" CREATED="1483581821347" HGAP="13" ID="ID_1893571865" MODIFIED="1483581962712" TEXT="Review Lebenszyklus" VSHIFT="8">
-<node COLOR="#338800" CREATED="1481510231986" ID="ID_45803267" MODIFIED="1482530111326" TEXT="dort SessionCommandService instantiieren">
 <icon BUILTIN="button_ok"/>
-<node CREATED="1481769089234" ID="ID_1152351588" MODIFIED="1481769124140" TEXT="lebt in der DispatcherLoop">
+<node CREATED="1483581829682" ID="ID_1563520696" MODIFIED="1483581904481" TEXT="alle Subsys-Operationen auf Semantik durchgepr&#xfc;ft">
+<icon BUILTIN="button_ok"/>
+</node>
+<node CREATED="1483581852071" ID="ID_1139549600" MODIFIED="1483581902223" TEXT="1/2017: Locking und Concurrency erscheint sauber behandelt">
+<icon BUILTIN="button_ok"/>
+</node>
+</node>
+</node>
+<node COLOR="#338800" CREATED="1481510231986" HGAP="23" ID="ID_45803267" MODIFIED="1483581960654" TEXT="dort SessionCommandService instantiieren" VSHIFT="4">
+<icon BUILTIN="button_ok"/>
+<node CREATED="1481769089234" ID="ID_1152351588" MODIFIED="1483582218575" TEXT="lebt in der DispatcherLoop">
 <arrowlink COLOR="#4aff51" DESTINATION="ID_1364443356" ENDARROW="Default" ENDINCLINATION="10;30;" ID="Arrow_ID_870320696" STARTARROW="Default" STARTINCLINATION="-221;-88;"/>
+<icon BUILTIN="info"/>
 </node>
 <node CREATED="1481769144226" ID="ID_1609251574" MODIFIED="1482532883149" TEXT="mu&#xdf; Service-API extrahieren">
 <icon BUILTIN="button_ok"/>
 </node>
-<node CREATED="1481777039358" ID="ID_395609084" MODIFIED="1481777050096" TEXT="Shutdown sauber regeln">
+<node CREATED="1481777039358" ID="ID_395609084" MODIFIED="1483582212694" TEXT="Shutdown sauber regeln">
+<icon BUILTIN="button_ok"/>
 <node CREATED="1481777054755" ID="ID_1551225439" MODIFIED="1481778421924">
 <richcontent TYPE="NODE"><html>
   <head>
@@ -597,9 +621,38 @@
 </node>
 <node CREATED="1481777114203" ID="ID_1156396729" MODIFIED="1481777121886" TEXT="Lock, um Deadlock zu vermeitden"/>
 <node CREATED="1481777122306" ID="ID_1506554988" MODIFIED="1481777133445" TEXT="alles im dtor  -&gt; noexcept"/>
-<node BACKGROUND_COLOR="#eee5c3" COLOR="#990000" CREATED="1481777210447" ID="ID_1392452935" MODIFIED="1481778497944" TEXT="TODO: sicherstellen da&#xdf; join nicht blockt">
+<node CREATED="1481777210447" ID="ID_1392452935" MODIFIED="1483582206339" TEXT="sicherstellen da&#xdf; shutdown nicht blockt">
+<richcontent TYPE="NOTE"><html>
+  <head>
+    
+  </head>
+  <body>
+    <p>
+      1/2017 Review durchgef&#252;hrt und Logik &#252;berarbeitet.
+    </p>
+    <p>
+      Einziger Risikofaktor ist nun, wenn beim Schlie&#223;en des SessionCommand-Intertfaces
+    </p>
+    <p>
+      oder beim Signalisieren an den Thread eine Exception fliegt; denn dann loggen wir zwar,
+    </p>
+    <p>
+      aber die Shutdown-R&#252;ckmeldung kommt u.U niemals an, und damit bleiben wir
+    </p>
+    <p>
+      am Ende von main() einfach h&#228;ngen.
+    </p>
+    <p>
+      
+    </p>
+    <p>
+      <i>Ich halte diese F&#228;lle aber f&#252;r in der Praxis nicht relevant,</i>&#160; und verzichte daher auf eine Spezialbehandlung
+    </p>
+  </body>
+</html>
+</richcontent>
 <arrowlink COLOR="#5a97df" DESTINATION="ID_1789585729" ENDARROW="Default" ENDINCLINATION="6300;584;" ID="Arrow_ID_1930701586" STARTARROW="None" STARTINCLINATION="8475;0;"/>
-<icon BUILTIN="flag-yellow"/>
+<icon BUILTIN="button_ok"/>
 </node>
 </node>
 </node>