Changeset 2294

Timestamp:

Aug 21, 2005, 11:03:21 AM (20 years ago)

Author:

bird

Message:

Tried to workaround the race conditions in libc_Back_safesemEvSleep/Wakeup.
These might affect SysV sems and shm.

Location:

trunk/src/emx

Files:

• ChangeLog.LIBC (modified) (1 diff, 1 prop)
• include/InnoTekLIBC/backend.h (modified) (1 diff, 1 prop)
• src/lib/sys/safesems.c (modified) (8 diffs, 1 prop)
• src/lib/sys/sysv_sem.c (modified) (5 diffs, 1 prop)
• src/lib/sys/sysv_shm.c (modified) (2 diffs, 1 prop)

trunk/src/emx/ChangeLog.LIBC

@@ -2 +2 @@
 
 TODO: open replace on RAMFS fails with error 32!
+
+2005-08-21: knut st. osmundsen <bird-gccos2-spam@anduin.net>
+    - libc:
+        o Tried to workaround the race conditions in __libc_Back_safesemEvSleep/Wakeup.
+          These might affect SysV sems and shm.
 
 2005-08-20: Lorne Sunley <lsunley@mb.sympatico.  >

trunk/src/emx/include/InnoTekLIBC/backend.h

@@ -1063 +1063 @@
  * @returns Negative error code (errno.h) on failure.
  * @param   hev         Event semaphore to post.
- */
-int __libc_Back_safesemEvWakeup(uintptr_t hev);
+ * @param   hmtx        The semaphore protecting hev (see __libc_Back_safesemEvSleep).
+ *                      The caller should own this semaphore, it's purpose is debug strictness only!
+ */
+int __libc_Back_safesemEvWakeup(uintptr_t hev, uintptr_t hmtx);
 
 

trunk/src/emx/src/lib/sys/safesems.c

@@ -32 +32 @@
 #define INCL_DOSEXCEPTIONS
 #define INCL_DOSERRORS
+#define INCL_DOSPROCESS
 #define INCL_FSMACROS
 #define INCL_EXAPIS
@@ -37 +38 @@
 #include <sys/errno.h>
 #include "syscalls.h"
+#include <emx/umalloc.h>
 #include <InnoTekLIBC/thread.h>
 #include <InnoTekLIBC/backend.h>
+#include <InnoTekLIBC/sharedpm.h>
 #define __LIBC_LOG_GROUP    __LIBC_LOG_GRP_BACK_IPC
 #include <InnoTekLIBC/logstrict.h>
+
+
+/*******************************************************************************
+*   Structures and Typedefs                                                    *
+*******************************************************************************/
+/**
+ * Event semaphore tracking structure.
+ *
+ * The event semaphore business is difficult because the lack of 
+ * atomic mutex release + event wait apis in OS/2. We have to 
+ * jump around the place to get this working nearly safly...
+ */      
+typedef struct __LIBC_SAFESEMEV
+{
+    /** The event semaphore. */
+    HEV                 hev;
+    /** Number of threads which are supposed to be blocking on the above event semaphore. */
+    uint32_t volatile   cWaiters;
+    /** Number of processes using the semaphore. */
+    uint32_t volatile   cUsers;
+    /** Set if the semaphore is shared. 
+     * If shared this structure is allocated from SPM, else it's from the heap. */
+    unsigned            fShared;
+#ifdef __LIBC_STRICT
+    /** The mutex semaphore used to protect the event semaphore. 
+     * Strict builds only. */
+    uintptr_t           hmtx;
+#endif
+} __LIBC_SAFESEMEV, *__LIBC_PSAFESEMEV;
+
+
+/**
+ * Arguments to semEvSleepSignalCallback().
+ */
+struct SignalArgs
+{
+    /** Set if pfnComplete have already been executed. */
+    int volatile            fDone;
+    /** Number of attempts at doing it. (in case of crashes in pfnComplete) */
+    int volatile            cTries;
+    /** Callback to execute. */
+    void                  (*pfnComplete)(void *pvUser);
+    /** User arg. */
+    void                   *pvUser;
+    /** The old priority - to be restored when we're unblocked. */
+    ULONG                   ulOldPri;
+    /** Pointer to the semaphore structure. */
+    __LIBC_PSAFESEMEV       pEv;
+};
 
 
@@ -206 +258 @@
 int __libc_Back_safesemEvCreate(uintptr_t *phev, int fShared)
 {
-    FS_VAR_SAVE_LOAD();
-    HEV hev = NULLHANDLE;
-    int rc = DosCreateEventSemEx(NULL, &hev, fShared ? DC_SEM_SHARED : 0, FALSE);
-    FS_RESTORE();
-    if (rc)
-        return -__libc_native2errno(rc);
-    *phev = hev;
-    return 0;
+    LIBCLOG_ENTER("phev=%p fShared=%d\n", (void *)phev, fShared);
+    int rc;
+    __LIBC_PSAFESEMEV pEv = NULL;
+    FS_VAR_SAVE_LOAD();
+    if (fShared)
+    {
+        /*
+         * Allocate from SPM, be very careful.
+         */
+        __LIBC_SPMXCPTREGREC RegRec;
+        rc = __libc_spmLock(&RegRec, NULL);
+        if (!rc)
+        {
+            pEv = __libc_spmAllocLocked(sizeof(*pEv));
+            if (pEv)
+            {
+                pEv->hev = NULLHANDLE;
+                pEv->cWaiters = 0;
+                pEv->cUsers = 1;
+                pEv->fShared = fShared;
+#ifdef __LIBC_STRICT
+                pEv->hmtx = 0;
+#endif 
+            
+                rc = DosCreateEventSemEx(NULL, &pEv->hev, fShared ? DC_SEM_SHARED : 0, FALSE);
+                if (rc)
+                {
+                    __libc_spmFree(pEv);
+                    rc = -__libc_native2errno(rc);
+                }
+            }
+            else
+                rc = -ENOMEM;
+            __libc_spmUnlock(&RegRec);
+        }
+    }
+    else
+    {
+        /*
+         * Allocate from high mem heap.
+         */
+        pEv = _hmalloc(sizeof(*pEv));
+        if (pEv)
+        {
+            pEv->hev = NULLHANDLE;
+            pEv->cWaiters = 0;
+            pEv->cUsers = 1;
+            pEv->fShared = fShared;
+#ifdef __LIBC_STRICT
+            pEv->hmtx = 0;
+#endif 
+
+            rc = DosCreateEventSemEx(NULL, &pEv->hev, fShared ? DC_SEM_SHARED : 0, FALSE);
+            if (rc)
+            {
+                free(pEv);
+                rc = -__libc_native2errno(rc);
+            }
+        }
+        else
+            rc = -ENOMEM;
+    }
+    FS_RESTORE();
+
+    if (!rc)
+        *phev = (uintptr_t)pEv;
+    LIBCLOG_RETURN_MSG(rc, "ret %d *phev=%#x\n", rc, *phev);
 }
 
@@ -226 +337 @@
 int __libc_Back_safesemEvOpen(uintptr_t hev)
 {
-    FS_VAR_SAVE_LOAD();
-    HMTX hevOS2 = hev;
-    int rc = DosOpenEventSemEx(NULL, &hevOS2);
-    FS_RESTORE();
-    if (rc)
-        return -__libc_native2errno(rc);
-    return 0;
+    LIBCLOG_ENTER("hev=%#x\n", hev);
+    __LIBC_PSAFESEMEV pEv = (__LIBC_PSAFESEMEV)hev;
+    FS_VAR_SAVE_LOAD();
+    int rc = DosOpenEventSemEx(NULL, &pEv->hev);
+    FS_RESTORE();
+    if (!rc)
+        __atomic_increment_u32(&pEv->cUsers);
+    else
+        rc = -__libc_native2errno(rc);
+    LIBCLOG_RETURN_INT(rc);
 }
 
@@ -245 +359 @@
 int __libc_Back_safesemEvClose(uintptr_t hev)
 {
-    FS_VAR_SAVE_LOAD();
-    int rc = DosCloseEventSemEx(hev);
-    FS_RESTORE();
-    if (rc)
-        return -__libc_native2errno(rc);
-    return 0;
-}
-
-
-
-/**
- * Arguments to semEvSleepSignalCallback().
- */
-struct SignalArgs
-{
-    /** Set if pfnComplete have already been executed. */
-    volatile int            fDone;
-    /** Number of attempts at doing it. (in case of crashes in pfnComplete) */
-    volatile int            cTries;
-    /** Callback to execute. */
-    void                  (*pfnComplete)(void *pvUser);
-    /** User arg. */
-    void                   *pvUser;
-};
+    LIBCLOG_ENTER("hev=%#x\n", hev);
+    __LIBC_PSAFESEMEV pEv = (__LIBC_PSAFESEMEV)hev;
+    FS_VAR_SAVE_LOAD();
+    int rc = DosCloseEventSemEx(pEv->hev);
+    FS_RESTORE();
+    if (!rc)
+    {
+        if (    pEv->cUsers
+            &&  !__atomic_decrement_u32(&pEv->cUsers))
+        {
+            pEv->hev = NULLHANDLE;
+            if (pEv->fShared)
+                __libc_spmFree(pEv);
+            else
+                free(pEv);
+        }
+    }
+    else
+        rc = -__libc_native2errno(rc);
+    LIBCLOG_RETURN_INT(rc);
+}
+
 
 /**
  * Signal notification callback.
+ *
+ * This will call the completion handler and restore the thread priority before
+ * any signal is executed. We're protect by the signal mutex/must-complete here.
  */
 static void semEvSleepSignalCallback(int iSignal, void *pvUser)
 {
+    LIBCLOG_ENTER("iSignal=%d pvUser=%p\n", iSignal, pvUser);
     struct SignalArgs *pArgs = (struct SignalArgs *)pvUser;
+
     if (!pArgs->fDone && pArgs->cTries++ < 5)
     {
         pArgs->pfnComplete(pArgs->pvUser);
+        int rc = DosSetPriority(PRTYS_THREAD, pArgs->ulOldPri >> 8, pArgs->ulOldPri & 0xff, 0);
+        if (rc)
+            __libc_Back_panic(0, NULL, "DosSetPriority(PRTYS_THREAD, %x, %x, 0) -> %d\n", 
+                              (unsigned)pArgs->ulOldPri >> 8, (unsigned)pArgs->ulOldPri & 0xff, rc);
         pArgs->fDone = 1;
     }
-}
+    LIBCLOG_RETURN_VOID();
+}
+
 
 /**
@@ -300 +423 @@
 {
     LIBCLOG_ENTER("hev=%#x hmtx=%#x pfnComplete=%p pvUser=%p\n", hev, hmtx, (void *)pfnComplete, pvUser);
-
-    /*
-     * Setup signal notification.
-     */
+    __LIBC_PSAFESEMEV pEv = (__LIBC_PSAFESEMEV)hev;
+
+#ifdef __LIBC_STRICT
+    /*
+     * Keep track of hmtx.
+     */
+    if (pEv->hmtx)
+        LIBC_ASSERTM(pEv->hmtx == hmtx, "pEv->hmtx=%#x hmtx=%#x\n", pEv->hmtx, hmtx);
+    else
+        pEv->hmtx = hmtx;
+#endif 
+
+    /*
+     * Setup signal notification callback.
+     */
+    int rc;
     __LIBC_PTHREAD pThrd = __libc_threadCurrentNoAuto();
-    int rc;
     if (pThrd)
     {
+        FS_VAR_SAVE_LOAD();
+        PTIB    pTib;
+        PPIB    pPib;
+        DosGetInfoBlocks(&pTib, &pPib);
+    
         struct SignalArgs Args;
+        Args.ulOldPri       = pTib->tib_ptib2->tib2_ulpri;
         Args.fDone          = 0;
         Args.cTries         = 0;
         Args.pfnComplete    = pfnComplete;
         Args.pvUser         = pvUser;
+    
         pThrd->pvSigCallbackUser = &Args;
         pThrd->pfnSigCallback    = semEvSleepSignalCallback;
-
+    
         /*
-         * Reset the event semaphore.
+         * Raise priority to time critical to increase our chances of actually getting 
+         * blocked before something bad like rescheduling or signaling strikes us.
          */
-        ULONG ulIgnore;
-        FS_VAR_SAVE_LOAD();
-        rc = DosResetEventSem(hev, &ulIgnore);
-        if (!rc || rc == ERROR_ALREADY_RESET)
+        rc = DosSetPriority(PRTYS_THREAD, PRTYC_TIMECRITICAL, 31, 0);
+        LIBC_ASSERTM(!rc, "DosSetPriority(PRTYS_THREAD, PRTYC_TIMECRITICAL, 31, 0) -> %d\n", rc);
+        if (rc && (Args.ulOldPri >> 8) != PRTYC_TIMECRITICAL)
+        {
+            rc = DosSetPriority(PRTYS_THREAD, PRTYC_TIMECRITICAL, 0, 0);
+            LIBC_ASSERTM(!rc, "DosSetPriority(PRTYS_THREAD, PRTYC_TIMECRITICAL, 0, 0) -> %d\n", rc);
+        }
+    
+        /*
+         * Release the sempahore and exit the must complete section.
+         */
+        __atomic_increment_u32(&pEv->cWaiters);
+        rc = __libc_Back_safesemMtxUnlock(hmtx);
+        if (!rc)
         {
             /*
-             * Release the sempahore and exit the must complete section.
+             * Now, lets block until something happens.
              */
-            rc = __libc_Back_safesemMtxUnlock(hmtx);
-            if (!rc)
+            int rc2 = 0;
+            if (!Args.fDone)
+                rc2 = DosWaitEventSem(pEv->hev, SEM_INDEFINITE_WAIT);
+    
+            /*
+             * Reclaim the ownership of the sempahore (w/must-complete) and deregister 
+             * the signal notification callback before we check if we was interrupted or not during the wait.
+             */
+            __atomic_decrement_u32(&pEv->cWaiters);
+            LIBCLOG_MSG("woke up - rc2=%d\n", rc2);
+            rc = __libc_Back_safesemMtxLock(hmtx);
+            pThrd->pfnSigCallback = NULL;
+            pThrd->pvSigCallbackUser = NULL;
+            if (!Args.fDone)
             {
-                /*
-                 * Now, lets wait.
+                /* 
+                 * Not interrupted.
+                 * Check for errors, do completion and restore priority. 
                  */
-                DosWaitEventSem(hev, SEM_INDEFINITE_WAIT);
-
-                /*
-                 * Regain access to the semaphore and must complete section
-                 * before checking if we got an interrupt or not.
-                 */
-                rc = __libc_Back_safesemMtxLock(hmtx);
-                if (!Args.fDone)
-                    pfnComplete(pvUser);
-                else if (!rc)
-                    rc = -EINTR;
+                if (rc2)
+                    rc = -__libc_native2errno(rc2);
+                pfnComplete(pvUser);
+                rc2 = DosSetPriority(PRTYS_THREAD, Args.ulOldPri >> 8, Args.ulOldPri & 0xff, 0);
+                if (rc2)
+                    __libc_Back_panic(0, NULL, "DosSetPriority(PRTYS_THREAD, %x, %x, 0) -> %d\n", 
+                                      (unsigned)Args.ulOldPri >> 8, (unsigned)Args.ulOldPri & 0xff, rc2);
             }
+            else if (!rc)
+                rc = -EINTR;
+    
+            /*
+             * Reset the event semaphore.
+             */
+            ULONG ulIgnore;
+            rc2 = DosResetEventSem(pEv->hev, &ulIgnore);
+            LIBC_ASSERTM(!rc2 && ERROR_ALREADY_RESET, "DosResetEventSem(%#lx,)->%d\n", pEv->hev, rc2);
         }
-        else
-            rc = -__libc_native2errno(rc);
         FS_RESTORE();
     }
@@ -363 +531 @@
  * @returns Negative error code (errno.h) on failure.
  * @param   hev         Event semaphore to post.
- */
-int __libc_Back_safesemEvWakeup(uintptr_t hev)
-{
-    LIBCLOG_ENTER("hev=%#x\n", hev);
-    FS_VAR_SAVE_LOAD();
-    int rc = DosPostEventSem(hev);
-    FS_RESTORE();
-    if (rc)
+ * @param   hmtx        The semaphore protecting hev (see __libc_Back_safesemEvSleep).
+ *                      The caller should own this semaphore, it's purpose is debug strictness only!
+ */
+int __libc_Back_safesemEvWakeup(uintptr_t hev, uintptr_t hmtx)
+{
+    LIBCLOG_ENTER("hev=%#x hmtx=%#x\n", hev, hmtx);
+    __LIBC_PSAFESEMEV pEv = (__LIBC_PSAFESEMEV)hev;
+    FS_VAR_SAVE_LOAD();
+    int rc;
+
+#ifdef __LIBC_STRICT
+    /*
+     * Check hmtx.
+     */
+    LIBC_ASSERTM(!pEv->hmtx || pEv->hmtx == hmtx, "pEv->hmtx=%#x hmtx=%#x\n", pEv->hmtx, hmtx);
+    ULONG   cNesting;
+    PID     pid;
+    TID     tid;
+    rc = DosQueryMutexSem(hmtx, &pid, &tid, &cNesting);
+    LIBC_ASSERTM(!rc, "DosQueryMutexSem(%#x,,,) -> %d\n", hmtx, rc);
+    if (!rc)
+        LIBC_ASSERTM(pid == fibGetPid() && tid == fibGetTid(),
+                     "pid=%d fibGetPid()->%d  tid=%d fibGetTid()->%d\n", (int)pid, fibGetPid(), (int)tid, fibGetTid());
+#endif 
+
+    /*
+     * Post it.
+     */
+    rc = DosPostEventSem(hev);
+    if (!rc)
+    {
+        if (!rc && pEv->cWaiters)
+        {
+            /* give the waiting threads a fair chance to get past the DosWaitEventSem() call. */
+            unsigned cYields = 3;
+            do
+            {
+                DosSleep(0);
+                LIBCLOG_MSG("cWaiters=%d cYields=%d\n", pEv->cWaiters, cYields);
+            } while (pEv->cWaiters && --cYields > 0);
+        }
+    }
+    else
     {
         if (rc == ERROR_ALREADY_POSTED || rc == ERROR_TOO_MANY_POSTS)
@@ -377 +580 @@
             rc = -__libc_native2errno(rc);
     }
+    FS_RESTORE();
     LIBCLOG_RETURN_INT(rc);
 }

trunk/src/emx/src/lib/sys/sysv_sem.c

@@ -628 +628 @@
                 semundo_clear(semid, -1);
                 SEMUNDO_UNLOCK();
-                __libc_Back_safesemEvWakeup(semaptr->hev);
+                __libc_Back_safesemEvWakeup(semaptr->hev, semaptr->hmtx);
                 break;
 
@@ -742 +742 @@
                 semundo_clear(semid, semnum);
                 SEMUNDO_UNLOCK();
-                __libc_Back_safesemEvWakeup(semaptr->hev);
+                __libc_Back_safesemEvWakeup(semaptr->hev, semaptr->hmtx);
                 break;
 
@@ -778 +778 @@
                 semundo_clear(semid, -1);
                 SEMUNDO_UNLOCK();
-                __libc_Back_safesemEvWakeup(semaptr->hev);
+                __libc_Back_safesemEvWakeup(semaptr->hev, semaptr->hmtx);
                 break;
 
@@ -1208 +1208 @@
         if (do_wakeup) {
                 DPRINTF(("semop:  doing wakeup\n"));
-                __libc_Back_safesemEvWakeup(semaptr->hev);
+                __libc_Back_safesemEvWakeup(semaptr->hev, semaptr->hmtx);
                 DPRINTF(("semop:  back from wakeup\n"));
         }
@@ -1298 +1298 @@
                                 semaptr->u.sem_base[semnum].semval += adjval;
 
-                        __libc_Back_safesemEvWakeup(semaptr->hev);
+                        __libc_Back_safesemEvWakeup(semaptr->hev, semaptr->hmtx);
                         DPRINTF(("semexit:  back from wakeup\n"));
                     skip:

trunk/src/emx/src/lib/sys/sysv_shm.c

@@ -286 +286 @@
         pGlobals->shm_nused--;
         shmseg->shm_perm.mode = SHMSEG_FREE;
-        __libc_Back_safesemEvWakeup(pGlobals->hevDaemon);
+        __libc_Back_safesemEvWakeup(pGlobals->hevDaemon, pGlobals->hmtx);
 }
 
@@ -314 +314 @@
                 pGlobals->shm_last_free = segnum;
         }
-        __libc_Back_safesemEvWakeup(pGlobals->hevDaemon);
+        __libc_Back_safesemEvWakeup(pGlobals->hevDaemon, pGlobals->hmtx);
         return (0);
 }