Fix race between two phase algorithm preSuspendBarrierand commit_begin

There is race between two phase algorithm preSuspendBarrier() and two
phase commit commit_begin().

T1 thread #1: setCkptPending()
T2               thread mpickpt#2: checks isCkptPending() then stop(comm)
T3              thread mpickpt#2: stop(comm) sets state to PHASE_1
T4              thread mpickpt#2: while isCkptPending() loop
T5 thread #1 calls waitForNewStateAfter(PHASE_1)

This change adds a timeout when waiting state change. When the
preSuspend wait for PHASE_1 state change times out, it would
set response to FREE_PASS.

Co-authored-by: Dahong Li <root@cori.nersc.gov>

Author: dahongli

mpi-proxy-split/two-phase-algo.cpp

@@ -230,8 +230,10 @@ TwoPhaseAlgo::preSuspendBarrier(query_t query)
     case INTENT:
       setCkptPending();
       if (getCurrState() == PHASE_1) {
-        // If in PHASE_1, wait for us to finish doing IN_CS
-        if (waitForNewStateAfter(PHASE_1) != IN_CS) {
+	st = waitForNewStateAfter(PHASE_1, 100 /* timeout ms*/);
+	if (st == PHASE_1) break;
+        // Wait for us to finish doing IN_CS
+        if (st != IN_CS) {
           break;
         } else {
           while (waitForNewStateAfter(IN_CS) != IN_CS);
@@ -299,24 +301,35 @@ TwoPhaseAlgo::stop(MPI_Comm comm)
   // INVARIANT: Ckpt should be pending when we get here
   JASSERT(isCkptPending());
   setCurrState(PHASE_1);
-
   while (isCkptPending() && !phase1_freepass) {
     sleep(1);
   }
   phase1_freepass = false;
 }
 
 phase_t
-TwoPhaseAlgo::waitForNewStateAfter(phase_t oldState)
+TwoPhaseAlgo::waitForNewStateAfter(phase_t oldState, int timeout_ms)
 {
   lock_t lock(_phaseMutex);
-  // The user thread will notify us if transition to any of these states
-  _phaseCv.wait(lock, [this, oldState]
+  if (timeout_ms) {
+    // Since _phaseCv is a condition var, it should be prepared for spurious wakeups.
+    // So, waking it up on a timeout is harmless.
+    _phaseCv.wait_for(lock, std::chrono::milliseconds(timeout_ms),
+                      [this, oldState]
                       { return _currState != oldState &&
                         ( _currState == IN_TRIVIAL_BARRIER ||
                           _currState == PHASE_1 ||
                           _currState == IN_CS ||
                           _currState == IS_READY); });
+  } else {
+    // The user thread will notify us if transition to any of these states
+    _phaseCv.wait(lock, [this, oldState]
+                        { return _currState != oldState &&
+                          ( _currState == IN_TRIVIAL_BARRIER ||
+                            _currState == PHASE_1 ||
+                            _currState == IN_CS ||
+                            _currState == IS_READY); });
+  }
   return _currState;
 }
 

mpi-proxy-split/two-phase-algo.h

@@ -162,7 +162,7 @@ namespace dmtcp_mpi
       void stop(MPI_Comm);
 
       // Wait until the state is changed to a new state
-      phase_t waitForNewStateAfter(phase_t oldState);
+      phase_t waitForNewStateAfter(phase_t oldState, int timeout_ms=0);
 
       // Sends the given message 'msg' (along with the given 'extraData') to
       // the coordinator