[SSI] Avoid race condition between Finished() and Event Dispatch (cau…

…ses SEGV).

src/XrdSsi/XrdSsiAtomics.hh

@@ -159,6 +159,8 @@ inline void   Lock(XrdSsiMutex *mutex)
 
 inline void   Lock(XrdSsiMutex &mutex) {Lock(&mutex);}
 
+inline void   Reset() {mtx = 0;}
+
 inline void UnLock() {if (mtx) {mtx->UnLock(); mtx = 0;}}
 
             XrdSsiMutexMon(XrdSsiMutex *mutex=0)

src/XrdSsi/XrdSsiEvent.cc

@@ -60,7 +60,7 @@ void XrdSsiEvent::AddEvent(XrdCl::XRootDStatus *st, XrdCl::AnyObject *resp)
 
 // Indicate there is pending event here
 //
-   DEBUG("Add event; isClear=" <<isClear <<" running=" <<running);
+   DEBUG("New event: isClear=" <<isClear <<" running=" <<running);
    isClear = false;
 
 // If the base object has no status then we need to set it and schedule
@@ -147,26 +147,46 @@ void XrdSsiEvent::ClrEvent(XrdSsiEvent::EventData *fdP)
 
 void XrdSsiEvent::DoIt()
 {
-   EventData myEvent, *edP = &myEvent;
+   EPNAME("RunEvent");
+   EventData *edP, myEvent;
+   int rc;
 
 // Process all of the events in our list. This is a tricky proposition because
-// the event executor may delete us when false is returned. To prevent double
-// frees and the like, we move out the event and work off a local copy.
+// the event executor may delete us upon return. Hence we do not directly use
+// any data members of this class, only copies. The return rc tells what to do.
+// rc > 0: terminate event processing and conclude in a normal fashion.
+// rc = 0: reflect next event.
+// rc < 0: immediately return as this object has become invalid.
 //
    evMutex.Lock();
 do{thisEvent.Move2(myEvent);
    lastEvent = 0;
    isClear   = true;
    evMutex.UnLock();
    edP = &myEvent;
-   while(edP && XeqEvent(edP->status, &edP->response)) {edP = edP->next;}
+
+   do {if ((rc = XeqEvent(edP->status, &edP->response)) != 0) break;
+       edP = edP->next;
+      } while(edP);
+
    ClrEvent(&myEvent);
-   if (edP) return;
+
+   if (rc)
+      {DEBUG("XeqEvent requested " <<(rc < 0 ? "halt" : "flush"));
+       if (rc > 0) break;
+       return;
+      }
+
    evMutex.Lock();
   } while(thisEvent.status);
 
-// We are done, indicate we are no longer running
+// Indicate we are no longer running
 //
    running = false;
    evMutex.UnLock();
+
+// The last thing we need to do is to tell the event handler that we are done
+// as it may decide to delete this object if no more events will occur.
+//
+   XeqEvFin();
 }

src/XrdSsi/XrdSsiEvent.hh

@@ -47,13 +47,16 @@ virtual void HandleResponse(XrdCl::XRootDStatus *status,
                             XrdCl::AnyObject *response)
                            {AddEvent(status, response);}
 
-virtual bool XeqEvent(XrdCl::XRootDStatus *st, XrdCl::AnyObject **resp) = 0;
+virtual int  XeqEvent(XrdCl::XRootDStatus *st, XrdCl::AnyObject **resp) = 0;
+
+virtual void XeqEvFin() = 0;
 
              XrdSsiEvent() : XrdJob(tident),  lastEvent(0),
                              running(false),  isClear(true)
                              {*tident = 0;}
 
             ~XrdSsiEvent() {if (!isClear) ClrEvent(&thisEvent);}
+
 protected:
 
 char   tident[24]; //"c %u#%u" with %u max 10 digits

src/XrdSsi/XrdSsiSessReal.cc

@@ -367,9 +367,9 @@ void XrdSsiSessReal::TaskFinished(XrdSsiTaskReal *tP)
 //
 
 
-// if we can shutdown, then unprovision which will drive a shutdown. Note
+// If we can shutdown, then unprovision which will drive a shutdown. Note
 // that Unprovision() returns without the sessMutex, otherwise we must
-// unlock it before we return.
+// unlock it before we return. A shutdown invalidates this object!
 //
    if (!inOpen)
       {if (!isHeld && !attBase) Unprovision();
@@ -403,8 +403,9 @@ void XrdSsiSessReal::UnHold(bool cleanup)
 /******************************************************************************/
 
 // Called with sessMutex locked and returns with it unlocked
+// Returns false if a shutdown occurred (i.e. session object no longer valid)
 
-void XrdSsiSessReal::Unprovision() // Called with sessMutex locked!
+bool XrdSsiSessReal::Unprovision() // Called with sessMutex locked!
 {
    EPNAME("Unprovision");
    XrdCl::XRootDStatus uStat;
@@ -417,21 +418,23 @@ void XrdSsiSessReal::Unprovision() // Called with sessMutex locked!
 // shutdown right away. Otherwise, try to close if successful the event
 // handler will do the shutdown, Otherwise, we do a Futterwacken dance.
 //
-   if (!epFile.IsOpen()) Shutdown(uStat, false);
+   if (!epFile.IsOpen()) {Shutdown(uStat, false); return false;}
       else {uStat = epFile.Close((XrdCl::ResponseHandler *)this);
-            if (!uStat.IsOK()) Shutdown(uStat, true);
+            if (!uStat.IsOK()) {Shutdown(uStat, true); return false;}
                else sessMutex.UnLock();
            }
+   return true;
 }
 
 /******************************************************************************/
 /*                              X e q E v e n t                               */
 /******************************************************************************/
 
-bool XrdSsiSessReal::XeqEvent(XrdCl::XRootDStatus *status,
+int  XrdSsiSessReal::XeqEvent(XrdCl::XRootDStatus *status,
                               XrdCl::AnyObject   **respP)
 {
-// Lock out mutex. Note that events like shutdown unlock the mutex
+// Lock out mutex. Note that events like shutdown unlock the mutex. The only
+// events handled here are open() and close().
 //
    sessMutex.Lock();
    XrdSsiTaskReal *ztP, *ntP, *tP = attBase;
@@ -441,7 +444,7 @@ bool XrdSsiSessReal::XeqEvent(XrdCl::XRootDStatus *status,
 //
    if (!inOpen)
       {Shutdown(*status, true); // sessMutex gets unlocked!
-       return false;
+       return -1; // This object no longer valid!
       }
 
 // We are no longer in open. However, if open encounetered an error then this
@@ -456,13 +459,13 @@ bool XrdSsiSessReal::XeqEvent(XrdCl::XRootDStatus *status,
    if (!tP)
       {if (isHeld)
           {sessMutex.UnLock();
-           return false;
+           return 1;
           }
        if (!status->IsOK()) Shutdown(*status, false);
-          else {if (!isHeld) Unprovision();
+          else {if (!isHeld) return (Unprovision() ? 1 : -1);
                    else sessMutex.UnLock();
                }
-       return false;
+       return 1; // Flush events and continue
       }
 
 // We are here because the open finally completed. If the open failed, then
@@ -475,7 +478,7 @@ bool XrdSsiSessReal::XeqEvent(XrdCl::XRootDStatus *status,
        do {tP->SchedError(&eInfo); tP = tP->attList.next;}
           while(tP != attBase);
        sessMutex.UnLock();
-       return false;
+       return 1;
       }
 
 // Obtain the endpoint name
@@ -498,5 +501,5 @@ bool XrdSsiSessReal::XeqEvent(XrdCl::XRootDStatus *status,
 // We are done, field the next event
 //
    sessMutex.UnLock();
-   return true;
+   return 0;
 }

src/XrdSsi/XrdSsiSessReal.hh

@@ -76,11 +76,13 @@ XrdSsiMutex     *MutexP() {return &sessMutex;}
 
         void     UnLock() {sessMutex.UnLock();}
 
-        void     Unprovision();
+        bool     Unprovision();
 
-        bool     XeqEvent(XrdCl::XRootDStatus *status,
+        int      XeqEvent(XrdCl::XRootDStatus *status,
                           XrdCl::AnyObject   **respP);
 
+        void     XeqEvFin() {}
+
                  XrdSsiSessReal(XrdSsiServReal *servP,
                                 const char     *sName,
                                 int             uent,