Fix a few bugs in named mutexes
authorKoundinya Veluri <kouvel@microsoft.com>
Fri, 20 May 2016 06:03:06 +0000 (23:03 -0700)
committerKoundinya Veluri <kouvel@microsoft.com>
Fri, 20 May 2016 16:53:38 +0000 (09:53 -0700)
- When using pthread mutexes, if the mutex tells us that it was abandoned (due to owning process abruptly shutting down), it was calling ReleaseMutex (which takes a handle) on the pthread mutex object. It shouldn't be releasing the lock anyway.
- When a locked mutex is closed, it wasn't being removed from the owner thread's list of owned named mutexes. This case should be treated as abandoned.
  - Since this could potentially happen from a different thread too, added the lock owner thread to the process data, added a critical section around list operations, and I now remove the process data from the owner thread's list on Close.
- Parent/child named mutex pal tests:
  - Cleaned up tests by refactoring the setup/cleanup code
  - Fixed a race where the parent may start a new child test before the previous child test closes its events
  - Added new tests for verifying that a mutex is locked after it reports that it was abandoned, and abandoning by closing a locked mutex

src/pal/src/include/pal/mutex.hpp
src/pal/src/include/pal/synchobjects.hpp
src/pal/src/synchmgr/synchmanager.cpp
src/pal/src/synchobj/mutex.cpp
src/pal/tests/palsuite/threading/NamedMutex/test1/namedmutex.cpp

index a420861..3d082d5 100644 (file)
@@ -146,6 +146,7 @@ private:
     HANDLE m_processLockHandle;
     int m_sharedLockFileDescriptor;
 #endif // !HAVE_FULLY_FEATURED_PTHREAD_MUTEXES
+    CorUnix::CPalThread *m_lockOwnerThread;
     NamedMutexProcessData *m_nextInThreadOwnedNamedMutexList;
 
 public:
@@ -166,6 +167,7 @@ public:
 
 private:
     NamedMutexSharedData *GetSharedData() const;
+    void SetLockOwnerThread(CorUnix::CPalThread *lockOwnerThread);
 public:
     NamedMutexProcessData *GetNextInThreadOwnedNamedMutexList() const;
     void SetNextInThreadOwnedNamedMutexList(NamedMutexProcessData *next);
index fc9705d..aa3a8f1 100644 (file)
@@ -114,6 +114,8 @@ namespace CorUnix
         Volatile<LONG>         m_lLocalSynchLockCount;
         Volatile<LONG>         m_lSharedSynchLockCount;
         LIST_ENTRY             m_leOwnedObjsList;
+
+        CRITICAL_SECTION       m_ownedNamedMutexListLock;
         NamedMutexProcessData *m_ownedNamedMutexListHead;
 
         ThreadNativeWaitData   m_tnwdNativeData;
@@ -172,7 +174,7 @@ namespace CorUnix
         void AddOwnedNamedMutex(NamedMutexProcessData *processData);
         void RemoveOwnedNamedMutex(NamedMutexProcessData *processData);
         NamedMutexProcessData *RemoveFirstOwnedNamedMutex();
-        bool OwnsNamedMutex(NamedMutexProcessData *processData) const;
+        bool OwnsNamedMutex(NamedMutexProcessData *processData);
 
         // The following methods provide access to the native wait lock for 
         // those implementations that need a lock to protect the support for 
index b37f2d4..473918c 100644 (file)
@@ -4016,6 +4016,7 @@ namespace CorUnix
             m_ownedNamedMutexListHead(nullptr)
     {
         InitializeListHead(&m_leOwnedObjsList);
+        InitializeCriticalSection(&m_ownedNamedMutexListLock);
 
 #ifdef SYNCHMGR_SUSPENSION_SAFE_CONDITION_SIGNALING
         m_lPendingSignalingCount = 0;
@@ -4025,6 +4026,7 @@ namespace CorUnix
 
     CThreadSynchronizationInfo::~CThreadSynchronizationInfo()
     {
+        DeleteCriticalSection(&m_ownedNamedMutexListLock);
         if (NULLSharedID != m_shridWaitAwakened)
         {
             RawSharedObjectFree(m_shridWaitAwakened);
@@ -4224,67 +4226,75 @@ namespace CorUnix
     void CThreadSynchronizationInfo::AddOwnedNamedMutex(NamedMutexProcessData *processData)
     {
         _ASSERTE(processData != nullptr);
-        _ASSERTE(this == &GetCurrentPalThread()->synchronizationInfo);
         _ASSERTE(processData->GetNextInThreadOwnedNamedMutexList() == nullptr);
 
+        EnterCriticalSection(&m_ownedNamedMutexListLock);
         processData->SetNextInThreadOwnedNamedMutexList(m_ownedNamedMutexListHead);
         m_ownedNamedMutexListHead = processData;
+        LeaveCriticalSection(&m_ownedNamedMutexListLock);
     }
 
     void CThreadSynchronizationInfo::RemoveOwnedNamedMutex(NamedMutexProcessData *processData)
     {
         _ASSERTE(processData != nullptr);
-        _ASSERTE(this == &GetCurrentPalThread()->synchronizationInfo);
 
+        EnterCriticalSection(&m_ownedNamedMutexListLock);
         if (m_ownedNamedMutexListHead == processData)
         {
             m_ownedNamedMutexListHead = processData->GetNextInThreadOwnedNamedMutexList();
             processData->SetNextInThreadOwnedNamedMutexList(nullptr);
-            return;
         }
-        for (NamedMutexProcessData
-                *previous = m_ownedNamedMutexListHead,
-                *current = previous->GetNextInThreadOwnedNamedMutexList();
-            current != nullptr;
-            previous = current, current = current->GetNextInThreadOwnedNamedMutexList())
+        else
         {
-            if (current == processData)
+            bool found = false;
+            for (NamedMutexProcessData
+                    *previous = m_ownedNamedMutexListHead,
+                    *current = previous->GetNextInThreadOwnedNamedMutexList();
+                current != nullptr;
+                previous = current, current = current->GetNextInThreadOwnedNamedMutexList())
             {
-                previous->SetNextInThreadOwnedNamedMutexList(current->GetNextInThreadOwnedNamedMutexList());
-                current->SetNextInThreadOwnedNamedMutexList(nullptr);
-                return;
+                if (current == processData)
+                {
+                    found = true;
+                    previous->SetNextInThreadOwnedNamedMutexList(current->GetNextInThreadOwnedNamedMutexList());
+                    current->SetNextInThreadOwnedNamedMutexList(nullptr);
+                    break;
+                }
             }
+            _ASSERTE(found);
         }
-        _ASSERTE(false);
+        LeaveCriticalSection(&m_ownedNamedMutexListLock);
     }
 
     NamedMutexProcessData *CThreadSynchronizationInfo::RemoveFirstOwnedNamedMutex()
     {
-        _ASSERTE(this == &GetCurrentPalThread()->synchronizationInfo);
-
+        EnterCriticalSection(&m_ownedNamedMutexListLock);
         NamedMutexProcessData *processData = m_ownedNamedMutexListHead;
         if (processData != nullptr)
         {
             m_ownedNamedMutexListHead = processData->GetNextInThreadOwnedNamedMutexList();
             processData->SetNextInThreadOwnedNamedMutexList(nullptr);
         }
+        LeaveCriticalSection(&m_ownedNamedMutexListLock);
         return processData;
     }
 
-    bool CThreadSynchronizationInfo::OwnsNamedMutex(NamedMutexProcessData *processData) const
+    bool CThreadSynchronizationInfo::OwnsNamedMutex(NamedMutexProcessData *processData)
     {
-        _ASSERTE(this == &GetCurrentPalThread()->synchronizationInfo);
-
+        EnterCriticalSection(&m_ownedNamedMutexListLock);
+        bool found = false;
         for (NamedMutexProcessData *current = m_ownedNamedMutexListHead;
             current != nullptr;
             current = current->GetNextInThreadOwnedNamedMutexList())
         {
             if (current == processData)
             {
-                return true;
+                found = true;
+                break;
             }
         }
-        return false;
+        LeaveCriticalSection(&m_ownedNamedMutexListLock);
+        return found;
     }
 
 #if SYNCHMGR_SUSPENSION_SAFE_CONDITION_SIGNALING
index 1eae81c..1977bbf 100644 (file)
@@ -877,7 +877,6 @@ MutexTryAcquireLockResult MutexHelpers::TryAcquireLock(pthread_mutex_t *mutex, D
         {
             int setConsistentResult = pthread_mutex_consistent(mutex);
             _ASSERTE(setConsistentResult == 0);
-            ReleaseMutex(mutex);
             return MutexTryAcquireLockResult::AcquiredLockButMutexWasAbandoned;
         }
 
@@ -1219,6 +1218,7 @@ NamedMutexProcessData::NamedMutexProcessData(
 #if !HAVE_FULLY_FEATURED_PTHREAD_MUTEXES
     m_sharedLockFileDescriptor(sharedLockFileDescriptor),
 #endif // !HAVE_FULLY_FEATURED_PTHREAD_MUTEXES
+    m_lockOwnerThread(nullptr),
     m_nextInThreadOwnedNamedMutexList(nullptr)
 {
     _ASSERTE(SharedMemoryManager::IsCreationDeletionProcessLockAcquired());
@@ -1242,9 +1242,28 @@ void NamedMutexProcessData::Close(bool isAbruptShutdown, bool releaseSharedData)
 
     // If the process is shutting down abruptly without having closed some mutexes, there could still be threads running with
     // active references to the mutex. So when shutting down abruptly, don't clean up any object or global process-local state.
-    if (!isAbruptShutdown && releaseSharedData)
+    if (!isAbruptShutdown)
     {
-        GetSharedData()->~NamedMutexSharedData();
+        CPalThread *lockOwnerThread = m_lockOwnerThread;
+        if (lockOwnerThread != nullptr)
+        {
+            // The mutex was not released before it was closed. If the lock is owned by the current thread, abandon the mutex.
+            // In both cases, clean up the owner thread's list of owned mutexes.
+            lockOwnerThread->synchronizationInfo.RemoveOwnedNamedMutex(this);
+            if (lockOwnerThread == GetCurrentPalThread())
+            {
+                Abandon();
+            }
+            else
+            {
+                m_lockOwnerThread = nullptr;
+            }
+        }
+
+        if (releaseSharedData)
+        {
+            GetSharedData()->~NamedMutexSharedData();
+        }
     }
 
 #if !HAVE_FULLY_FEATURED_PTHREAD_MUTEXES
@@ -1278,15 +1297,21 @@ NamedMutexSharedData *NamedMutexProcessData::GetSharedData() const
     return reinterpret_cast<NamedMutexSharedData *>(m_processDataHeader->GetSharedDataHeader()->GetData());
 }
 
-NamedMutexProcessData *NamedMutexProcessData::GetNextInThreadOwnedNamedMutexList() const
+void NamedMutexProcessData::SetLockOwnerThread(CorUnix::CPalThread *lockOwnerThread)
 {
+    _ASSERTE(lockOwnerThread == nullptr || lockOwnerThread == GetCurrentPalThread());
     _ASSERTE(GetSharedData()->IsLockOwnedByCurrentThread());
+
+    m_lockOwnerThread = lockOwnerThread;
+}
+
+NamedMutexProcessData *NamedMutexProcessData::GetNextInThreadOwnedNamedMutexList() const
+{
     return m_nextInThreadOwnedNamedMutexList;
 }
 
 void NamedMutexProcessData::SetNextInThreadOwnedNamedMutexList(NamedMutexProcessData *next)
 {
-    _ASSERTE(GetSharedData()->IsLockOwnedByCurrentThread());
     m_nextInThreadOwnedNamedMutexList = next;
 }
 
@@ -1488,7 +1513,9 @@ MutexTryAcquireLockResult NamedMutexProcessData::TryAcquireLock(DWORD timeoutMil
 
     sharedData->SetLockOwnerToCurrentThread();
     m_lockCount = 1;
-    GetCurrentPalThread()->synchronizationInfo.AddOwnedNamedMutex(this);
+    CPalThread *currentThread = GetCurrentPalThread();
+    SetLockOwnerThread(currentThread);
+    currentThread->synchronizationInfo.AddOwnedNamedMutex(this);
 
     if (sharedData->IsAbandoned())
     {
@@ -1516,6 +1543,7 @@ void NamedMutexProcessData::ReleaseLock()
     }
 
     GetCurrentPalThread()->synchronizationInfo.RemoveOwnedNamedMutex(this);
+    SetLockOwnerThread(nullptr);
     ActuallyReleaseLock();
 }
 
@@ -1527,6 +1555,7 @@ void NamedMutexProcessData::Abandon()
 
     sharedData->SetIsAbandoned(true);
     m_lockCount = 0;
+    SetLockOwnerThread(nullptr);
     ActuallyReleaseLock();
 }
 
index 7ee86e3..4e377f7 100644 (file)
@@ -23,6 +23,7 @@ const char *const ParentEventNamePrefix0 = "paltest_namedmutex_test1_pe0_";
 const char *const ParentEventNamePrefix1 = "paltest_namedmutex_test1_pe1_";
 const char *const ChildEventNamePrefix0 = "paltest_namedmutex_test1_ce0_";
 const char *const ChildEventNamePrefix1 = "paltest_namedmutex_test1_ce1_";
+const char *const ChildRunningEventNamePrefix = "paltest_namedmutex_test1_cr_";
 
 const char *const GlobalShmFilePathPrefix = "/tmp/.dotnet/shm/global/";
 
@@ -226,13 +227,97 @@ bool WaitForMutexToBeCreated(AutoCloseMutexHandle &m, const char *eventNamePrefi
 // YieldToParent() below control the releasing, waiting, and ping-ponging, to help create a deterministic path through the
 // parent and child tests while both are running concurrently.
 
-bool InitialWaitForParent(AutoCloseMutexHandle parentEvents[2])
+bool AcquireChildRunningEvent(AutoCloseMutexHandle &childRunningEvent)
 {
+    char name[MaxPathSize];
+    TestCreateMutex(childRunningEvent, BuildName(name, GlobalPrefix, ChildRunningEventNamePrefix));
+    TestAssert(WaitForSingleObject(childRunningEvent, FailTimeoutMilliseconds) == WAIT_OBJECT_0);
+    return true;
+}
+
+bool InitializeParent(AutoCloseMutexHandle parentEvents[2], AutoCloseMutexHandle childEvents[2])
+{
+    // Create parent events
+    char name[MaxPathSize];
+    for (int i = 0; i < 2; ++i)
+    {
+        TestCreateMutex(
+            parentEvents[i],
+            BuildName(name, GlobalPrefix, i == 0 ? ParentEventNamePrefix0 : ParentEventNamePrefix1),
+            true);
+        TestAssert(parentEvents[i] != nullptr);
+        TestAssert(GetLastError() != ERROR_ALREADY_EXISTS);
+    }
+
+    // Wait for the child to create and acquire locks on its events so that the parent can wait on them
+    TestAssert(WaitForMutexToBeCreated(childEvents[0], ChildEventNamePrefix0));
+    TestAssert(WaitForMutexToBeCreated(childEvents[1], ChildEventNamePrefix1));
+    return true;
+}
+
+bool UninitializeParent(AutoCloseMutexHandle parentEvents[2], bool releaseParentEvents = true)
+{
+    if (releaseParentEvents)
+    {
+        TestAssert(parentEvents[0].Release());
+        TestAssert(parentEvents[1].Release());
+    }
+
+    // Wait for the child to finish its test. Child tests will release and close 'childEvents' before releasing
+    // 'childRunningEvent', so after this wait, the parent process can freely start another child that will deterministically
+    // recreate the 'childEvents', which the next parent test will wait on, upon its initialization.
+    AutoCloseMutexHandle childRunningEvent;
+    TestAssert(AcquireChildRunningEvent(childRunningEvent));
+    TestAssert(childRunningEvent.Release());
+    return true;
+}
+
+bool InitializeChild(
+    AutoCloseMutexHandle &childRunningEvent,
+    AutoCloseMutexHandle parentEvents[2],
+    AutoCloseMutexHandle childEvents[2])
+{
+    TestAssert(AcquireChildRunningEvent(childRunningEvent));
+
+    // Create child events
+    char name[MaxPathSize];
+    for (int i = 0; i < 2; ++i)
+    {
+        TestCreateMutex(
+            childEvents[i],
+            BuildName(name, GlobalPrefix, i == 0 ? ChildEventNamePrefix0 : ChildEventNamePrefix1),
+            true);
+        TestAssert(childEvents[i] != nullptr);
+        TestAssert(GetLastError() != ERROR_ALREADY_EXISTS);
+    }
+
+    // Wait for the parent to create and acquire locks on its events so that the child can wait on them
+    TestAssert(WaitForMutexToBeCreated(parentEvents[0], ParentEventNamePrefix0));
+    TestAssert(WaitForMutexToBeCreated(parentEvents[1], ParentEventNamePrefix1));
+
+    // Parent/child tests start with the parent, so after initialization, wait for the parent to tell the child test to start
     TestAssert(WaitForSingleObject(parentEvents[0], FailTimeoutMilliseconds) == WAIT_OBJECT_0);
     TestAssert(parentEvents[0].Release());
     return true;
 }
 
+bool UninitializeChild(
+    AutoCloseMutexHandle &childRunningEvent,
+    AutoCloseMutexHandle parentEvents[2],
+    AutoCloseMutexHandle childEvents[2])
+{
+    // Release and close 'parentEvents' and 'childEvents' before releasing 'childRunningEvent' to avoid races, see
+    // UnitializeParent() for more info
+    TestAssert(childEvents[0].Release());
+    TestAssert(childEvents[1].Release());
+    childEvents[0].Close();
+    childEvents[1].Close();
+    parentEvents[0].Close();
+    parentEvents[1].Close();
+    TestAssert(childRunningEvent.Release());
+    return true;
+}
+
 bool YieldToChild(AutoCloseMutexHandle parentEvents[2], AutoCloseMutexHandle childEvents[2], int &ei)
 {
     TestAssert(parentEvents[ei].Release());
@@ -253,13 +338,6 @@ bool YieldToParent(AutoCloseMutexHandle parentEvents[2], AutoCloseMutexHandle ch
     return true;
 }
 
-bool FinalWaitForChild(AutoCloseMutexHandle childEvents[2], int ei)
-{
-    TestAssert(WaitForSingleObject(childEvents[ei], FailTimeoutMilliseconds) == WAIT_OBJECT_0);
-    TestAssert(childEvents[ei].Release());
-    return true;
-}
-
 ////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
 // Tests
 
@@ -360,20 +438,11 @@ bool HeaderMismatchTests()
 
 bool MutualExclusionTests_Parent()
 {
-    AutoCloseMutexHandle m, parentEvents[2], childEvents[2];
-    char name[MaxPathSize];
-    for (int i = 0; i < 2; ++i)
-    {
-        TestCreateMutex(
-            parentEvents[i],
-            BuildName(name, GlobalPrefix, i == 0 ? ParentEventNamePrefix0 : ParentEventNamePrefix1),
-            true);
-        TestAssert(parentEvents[i] != nullptr);
-        TestAssert(GetLastError() != ERROR_ALREADY_EXISTS);
-    }
-    TestAssert(WaitForMutexToBeCreated(childEvents[0], ChildEventNamePrefix0));
-    TestAssert(WaitForMutexToBeCreated(childEvents[1], ChildEventNamePrefix1));
+    AutoCloseMutexHandle parentEvents[2], childEvents[2];
+    TestAssert(InitializeParent(parentEvents, childEvents));
     int ei = 0;
+    char name[MaxPathSize];
+    AutoCloseMutexHandle m;
 
     TestCreateMutex(m, BuildName(name, GlobalPrefix, NamePrefix));
     TestAssert(m != nullptr);
@@ -400,41 +469,25 @@ bool MutualExclusionTests_Parent()
     TestAssert(WaitForSingleObject(m, static_cast<DWORD>(-1)) == WAIT_OBJECT_0); // lock the mutex with no timeout and release
     TestAssert(m.Release());
 
-    TestAssert(parentEvents[0].Release());
-    TestAssert(parentEvents[1].Release());
-    FinalWaitForChild(childEvents, ei);
+    UninitializeParent(parentEvents);
     return true;
 }
 
 DWORD MutualExclusionTests_Child(void *arg = nullptr)
 {
-    AutoCloseMutexHandle childEvents[2];
-    {
-        AutoCloseMutexHandle m, parentEvents[2];
-        char name[MaxPathSize];
-        for (int i = 0; i < 2; ++i)
-        {
-            TestCreateMutex(
-                childEvents[i],
-                BuildName(name, GlobalPrefix, i == 0 ? ChildEventNamePrefix0 : ChildEventNamePrefix1),
-                true);
-            TestAssert(childEvents[i] != nullptr);
-            TestAssert(GetLastError() != ERROR_ALREADY_EXISTS);
-        }
-        TestAssert(WaitForMutexToBeCreated(parentEvents[0], ParentEventNamePrefix0));
-        TestAssert(WaitForMutexToBeCreated(parentEvents[1], ParentEventNamePrefix1));
-        int ei = 0;
+    AutoCloseMutexHandle childRunningEvent, parentEvents[2], childEvents[2];
+    TestAssert(InitializeChild(childRunningEvent, parentEvents, childEvents));
+    int ei = 0;
+    char name[MaxPathSize];
+    AutoCloseMutexHandle m;
 
-        InitialWaitForParent(parentEvents);
-        TestCreateMutex(m, BuildName(name, GlobalPrefix, NamePrefix));
-        TestAssert(m != nullptr);
-        TestAssert(WaitForSingleObject(m, 0) == WAIT_OBJECT_0); // lock the mutex
-        YieldToParent(parentEvents, childEvents, ei); // parent attempts to lock/release, and fails
-        TestAssert(m.Release()); // release the lock
-    }
+    TestCreateMutex(m, BuildName(name, GlobalPrefix, NamePrefix));
+    TestAssert(m != nullptr);
+    TestAssert(WaitForSingleObject(m, 0) == WAIT_OBJECT_0); // lock the mutex
+    YieldToParent(parentEvents, childEvents, ei); // parent attempts to lock/release, and fails
+    TestAssert(m.Release()); // release the lock
 
-    TestAssert(childEvents[0].Release());
-    TestAssert(childEvents[1].Release());
+    UninitializeChild(childRunningEvent, parentEvents, childEvents);
     return 0;
 }
 
@@ -489,20 +542,11 @@ bool MutualExclusionTests()
 
 bool LifetimeTests_Parent()
 {
-    AutoCloseMutexHandle m, parentEvents[2], childEvents[2];
-    char name[MaxPathSize];
-    for (int i = 0; i < 2; ++i)
-    {
-        TestCreateMutex(
-            parentEvents[i],
-            BuildName(name, GlobalPrefix, i == 0 ? ParentEventNamePrefix0 : ParentEventNamePrefix1),
-            true);
-        TestAssert(parentEvents[i] != nullptr);
-        TestAssert(GetLastError() != ERROR_ALREADY_EXISTS);
-    }
-    TestAssert(WaitForMutexToBeCreated(childEvents[0], ChildEventNamePrefix0));
-    TestAssert(WaitForMutexToBeCreated(childEvents[1], ChildEventNamePrefix1));
+    AutoCloseMutexHandle parentEvents[2], childEvents[2];
+    TestAssert(InitializeParent(parentEvents, childEvents));
     int ei = 0;
+    char name[MaxPathSize];
+    AutoCloseMutexHandle m;
 
     TestCreateMutex(m, BuildName(name, GlobalPrefix, NamePrefix)); // create first reference to mutex
     TestAssert(m != nullptr);
@@ -522,48 +566,33 @@ bool LifetimeTests_Parent()
     TestAssert(YieldToChild(parentEvents, childEvents, ei)); // child closes second reference
     TestAssert(!TestFileExists(BuildGlobalShmFilePath(name, NamePrefix)));
 
-    TestAssert(parentEvents[0].Release());
-    TestAssert(parentEvents[1].Release());
-    FinalWaitForChild(childEvents, ei);
+    UninitializeParent(parentEvents);
     return true;
 }
 
 DWORD LifetimeTests_Child(void *arg = nullptr)
 {
-    AutoCloseMutexHandle childEvents[2];
-    {
-        AutoCloseMutexHandle m, parentEvents[2];
-        char name[MaxPathSize];
-        for (int i = 0; i < 2; ++i)
-        {
-            TestCreateMutex(
-                childEvents[i],
-                BuildName(name, GlobalPrefix, i == 0 ? ChildEventNamePrefix0 : ChildEventNamePrefix1),
-                true);
-            TestAssert(childEvents[i] != nullptr);
-            TestAssert(GetLastError() != ERROR_ALREADY_EXISTS);
-        }
-        TestAssert(WaitForMutexToBeCreated(parentEvents[0], ParentEventNamePrefix0));
-        TestAssert(WaitForMutexToBeCreated(parentEvents[1], ParentEventNamePrefix1));
-        int ei = 0;
+    AutoCloseMutexHandle childRunningEvent, parentEvents[2], childEvents[2];
+    TestAssert(InitializeChild(childRunningEvent, parentEvents, childEvents));
+    int ei = 0;
+    char name[MaxPathSize];
+    AutoCloseMutexHandle m;
 
-        InitialWaitForParent(parentEvents); // parent creates first reference to mutex
-        TestCreateMutex(m, BuildName(name, GlobalPrefix, NamePrefix)); // create second reference to mutex using CreateMutex
-        TestAssert(m != nullptr);
-        TestAssert(YieldToParent(parentEvents, childEvents, ei)); // parent closes first reference
-        m.Close(); // close second reference
+    // ... parent creates first reference to mutex
+    TestCreateMutex(m, BuildName(name, GlobalPrefix, NamePrefix)); // create second reference to mutex using CreateMutex
+    TestAssert(m != nullptr);
+    TestAssert(YieldToParent(parentEvents, childEvents, ei)); // parent closes first reference
+    m.Close(); // close second reference
 
-        TestAssert(YieldToParent(parentEvents, childEvents, ei)); // parent verifies, and creates first reference to mutex again
-        m = TestOpenMutex(BuildName(name, GlobalPrefix, NamePrefix)); // create second reference to mutex using OpenMutex
-        TestAssert(m != nullptr);
-        TestAssert(YieldToParent(parentEvents, childEvents, ei)); // parent closes first reference
-        m.Close(); // close second reference
+    TestAssert(YieldToParent(parentEvents, childEvents, ei)); // parent verifies, and creates first reference to mutex again
+    m = TestOpenMutex(BuildName(name, GlobalPrefix, NamePrefix)); // create second reference to mutex using OpenMutex
+    TestAssert(m != nullptr);
+    TestAssert(YieldToParent(parentEvents, childEvents, ei)); // parent closes first reference
+    m.Close(); // close second reference
 
-        TestAssert(YieldToParent(parentEvents, childEvents, ei)); // parent verifies
-    }
+    TestAssert(YieldToParent(parentEvents, childEvents, ei)); // parent verifies
 
-    TestAssert(childEvents[0].Release());
-    TestAssert(childEvents[1].Release());
+    UninitializeChild(childRunningEvent, parentEvents, childEvents);
     return 0;
 }
 
@@ -590,107 +619,138 @@ bool LifetimeTests()
     return true;
 }
 
+DWORD AbandonTests_Child_TryLock(void *arg = nullptr);
+
 bool AbandonTests_Parent()
 {
-    AutoCloseMutexHandle m, parentEvents[2], childEvents[2];
-    char name[MaxPathSize];
-    for (int i = 0; i < 2; ++i)
+    AutoCloseMutexHandle m;
     {
-        TestCreateMutex(
-            parentEvents[i],
-            BuildName(name, GlobalPrefix, i == 0 ? ParentEventNamePrefix0 : ParentEventNamePrefix1),
-            true);
-        TestAssert(parentEvents[i] != nullptr);
-        TestAssert(GetLastError() != ERROR_ALREADY_EXISTS);
+        AutoCloseMutexHandle parentEvents[2], childEvents[2];
+        TestAssert(InitializeParent(parentEvents, childEvents));
+        int ei = 0;
+        char name[MaxPathSize];
+
+        TestCreateMutex(m, BuildName(name, GlobalPrefix, NamePrefix));
+        TestAssert(m != nullptr);
+        TestAssert(YieldToChild(parentEvents, childEvents, ei)); // child locks mutex
+        TestAssert(parentEvents[0].Release());
+        TestAssert(parentEvents[1].Release()); // child sleeps for short duration and abandons the mutex
+        TestAssert(WaitForSingleObject(m, FailTimeoutMilliseconds) == WAIT_ABANDONED_0); // attempt to lock and see abandoned mutex
+
+        UninitializeParent(parentEvents, false /* releaseParentEvents */); // parent events are released above
     }
-    TestAssert(WaitForMutexToBeCreated(childEvents[0], ChildEventNamePrefix0));
-    TestAssert(WaitForMutexToBeCreated(childEvents[1], ChildEventNamePrefix1));
+
+    // Verify that the mutex lock is owned by this thread, by starting a new thread and trying to lock it
+    StartThread(AbandonTests_Child_TryLock);
+    {
+        AutoCloseMutexHandle parentEvents[2], childEvents[2];
+        TestAssert(InitializeParent(parentEvents, childEvents));
+        int ei = 0;
+
+        TestAssert(YieldToChild(parentEvents, childEvents, ei)); // child tries to lock mutex
+
+        UninitializeParent(parentEvents);
+    }
+
+    // Verify that the mutex lock is owned by this thread, by starting a new process and trying to lock it
+    StartProcess("AbandonTests_Child_TryLock");
+    AutoCloseMutexHandle parentEvents[2], childEvents[2];
+    TestAssert(InitializeParent(parentEvents, childEvents));
     int ei = 0;
 
-    TestCreateMutex(m, BuildName(name, GlobalPrefix, NamePrefix));
-    TestAssert(m != nullptr);
-    TestAssert(YieldToChild(parentEvents, childEvents, ei)); // child locks mutex
-    TestAssert(parentEvents[0].Release());
-    TestAssert(parentEvents[1].Release()); // child sleeps for short duration and abandons the mutex
-    TestAssert(WaitForSingleObject(m, FailTimeoutMilliseconds) == WAIT_ABANDONED_0); // attempt to lock and see abandoned mutex
+    TestAssert(YieldToChild(parentEvents, childEvents, ei)); // child tries to lock mutex
+
+    // Continue verification
     TestAssert(m.Release());
     TestAssert(WaitForSingleObject(m, FailTimeoutMilliseconds) == WAIT_OBJECT_0); // lock again to see it's not abandoned anymore
     TestAssert(m.Release());
 
-    FinalWaitForChild(childEvents, ei);
+    UninitializeParent(parentEvents, false /* releaseParentEvents */); // parent events are released above
     return true;
 }
 
 DWORD AbandonTests_Child_GracefulExit(void *arg = nullptr)
 {
-    AutoCloseMutexHandle childEvents[2];
+    AutoCloseMutexHandle childRunningEvent, parentEvents[2], childEvents[2];
+    TestAssert(InitializeChild(childRunningEvent, parentEvents, childEvents));
+    int ei = 0;
+    char name[MaxPathSize];
+    AutoCloseMutexHandle m;
+
+    // ... parent waits for child to lock mutex
+    TestCreateMutex(m, BuildName(name, GlobalPrefix, NamePrefix));
+    TestAssert(m != nullptr);
+    TestAssert(WaitForSingleObject(m, 0) == WAIT_OBJECT_0);
+    TestAssert(YieldToParent(parentEvents, childEvents, ei)); // parent waits on mutex
+    Sleep(500); // wait for parent to wait on mutex
+    m.Abandon(); // don't close the mutex
+
+    UninitializeChild(childRunningEvent, parentEvents, childEvents);
+    return 0;
+}
+
+DWORD AbandonTests_Child_GracefulExit_CloseBeforeRelease(void *arg = nullptr)
+{
+    AutoCloseMutexHandle childRunningEvent, parentEvents[2], childEvents[2];
+    TestAssert(InitializeChild(childRunningEvent, parentEvents, childEvents));
+    int ei = 0;
+    char name[MaxPathSize];
+    AutoCloseMutexHandle m;
+
+    // ... parent waits for child to lock mutex
+    TestCreateMutex(m, BuildName(name, GlobalPrefix, NamePrefix));
+    TestAssert(m != nullptr);
+    TestAssert(WaitForSingleObject(m, 0) == WAIT_OBJECT_0);
+    TestAssert(YieldToParent(parentEvents, childEvents, ei)); // parent waits on mutex
+    Sleep(500); // wait for parent to wait on mutex
+    m.Close(); // close mutex before releasing lock
+
+    UninitializeChild(childRunningEvent, parentEvents, childEvents);
+    return 0;
+}
+
+DWORD AbandonTests_Child_AbruptExit(void *arg = nullptr)
+{
+    DWORD currentPid = test_getpid();
+    TestAssert(currentPid != parentPid); // this test needs to run in a separate process
+
     {
-        AutoCloseMutexHandle m, parentEvents[2];
-        char name[MaxPathSize];
-        for (int i = 0; i < 2; ++i)
-        {
-            TestCreateMutex(
-                childEvents[i],
-                BuildName(name, GlobalPrefix, i == 0 ? ChildEventNamePrefix0 : ChildEventNamePrefix1),
-                true);
-            TestAssert(childEvents[i] != nullptr);
-            TestAssert(GetLastError() != ERROR_ALREADY_EXISTS);
-        }
-        TestAssert(WaitForMutexToBeCreated(parentEvents[0], ParentEventNamePrefix0));
-        TestAssert(WaitForMutexToBeCreated(parentEvents[1], ParentEventNamePrefix1));
+        AutoCloseMutexHandle childRunningEvent, parentEvents[2], childEvents[2];
+        TestAssert(InitializeChild(childRunningEvent, parentEvents, childEvents));
         int ei = 0;
+        char name[MaxPathSize];
+        AutoCloseMutexHandle m;
 
-        InitialWaitForParent(parentEvents); // parent waits for child to lock mutex
+        // ... parent waits for child to lock mutex
         TestCreateMutex(m, BuildName(name, GlobalPrefix, NamePrefix));
         TestAssert(m != nullptr);
         TestAssert(WaitForSingleObject(m, 0) == WAIT_OBJECT_0);
         TestAssert(YieldToParent(parentEvents, childEvents, ei)); // parent waits on mutex
         Sleep(500); // wait for parent to wait on mutex
         m.Abandon(); // don't close the mutex
+
+        UninitializeChild(childRunningEvent, parentEvents, childEvents);
     }
 
-    TestAssert(childEvents[0].Release());
-    TestAssert(childEvents[1].Release());
-    return 0; // abandon the mutex gracefully
+    TestAssert(test_kill(currentPid) == 0); // abandon the mutex abruptly
+    return 0;
 }
 
-DWORD AbandonTests_Child_AbruptExit(void *arg = nullptr)
+DWORD AbandonTests_Child_TryLock(void *arg)
 {
-    DWORD currentPid = test_getpid();
-    TestAssert(currentPid != parentPid); // this test needs to run in a separate process
-
-    {
-        AutoCloseMutexHandle childEvents[2];
-        {
-            AutoCloseMutexHandle m, parentEvents[2];
-            char name[MaxPathSize];
-            for (int i = 0; i < 2; ++i)
-            {
-                TestCreateMutex(
-                    childEvents[i],
-                    BuildName(name, GlobalPrefix, i == 0 ? ChildEventNamePrefix0 : ChildEventNamePrefix1),
-                    true);
-                TestAssert(childEvents[i] != nullptr);
-                TestAssert(GetLastError() != ERROR_ALREADY_EXISTS);
-            }
-            TestAssert(WaitForMutexToBeCreated(parentEvents[0], ParentEventNamePrefix0));
-            TestAssert(WaitForMutexToBeCreated(parentEvents[1], ParentEventNamePrefix1));
-            int ei = 0;
-
-            InitialWaitForParent(parentEvents); // parent waits for child to lock mutex
-            TestCreateMutex(m, BuildName(name, GlobalPrefix, NamePrefix));
-            TestAssert(m != nullptr);
-            TestAssert(WaitForSingleObject(m, 0) == WAIT_OBJECT_0);
-            TestAssert(YieldToParent(parentEvents, childEvents, ei)); // parent waits on mutex
-            Sleep(500); // wait for parent to wait on mutex
-            m.Abandon(); // don't close the mutex
-        }
+    AutoCloseMutexHandle childRunningEvent, parentEvents[2], childEvents[2];
+    TestAssert(InitializeChild(childRunningEvent, parentEvents, childEvents));
+    int ei = 0;
+    char name[MaxPathSize];
+    AutoCloseMutexHandle m;
 
-        TestAssert(childEvents[0].Release());
-        TestAssert(childEvents[1].Release());
-    }
+    // ... parent waits for child to lock mutex
+    TestCreateMutex(m, BuildName(name, GlobalPrefix, NamePrefix));
+    TestAssert(m != nullptr);
+    TestAssert(WaitForSingleObject(m, 0) == WAIT_TIMEOUT); // try to lock the mutex while the parent holds the lock
+    TestAssert(WaitForSingleObject(m, 500) == WAIT_TIMEOUT);
 
-    TestAssert(test_kill(currentPid) == 0); // abandon the mutex abruptly
+    UninitializeChild(childRunningEvent, parentEvents, childEvents);
     return 0;
 }
 
@@ -702,6 +762,12 @@ bool AbandonTests()
     TestAssert(StartProcess("AbandonTests_Child_GracefulExit"));
     TestAssert(AbandonTests_Parent());
 
+    // Abandon by graceful exit where the lock owner closes the mutex before releasing it, unblocks a waiter
+    TestAssert(StartThread(AbandonTests_Child_GracefulExit_CloseBeforeRelease));
+    TestAssert(AbandonTests_Parent());
+    TestAssert(StartProcess("AbandonTests_Child_GracefulExit_CloseBeforeRelease"));
+    TestAssert(AbandonTests_Parent());
+
     // Abandon by abrupt exit unblocks a waiter
     TestAssert(StartProcess("AbandonTests_Child_AbruptExit"));
     TestAssert(AbandonTests_Parent());
@@ -778,10 +844,18 @@ int __cdecl main(int argc, char **argv)
     {
         AbandonTests_Child_GracefulExit();
     }
+    else if (test_strcmp(argv[2], "AbandonTests_Child_GracefulExit_CloseBeforeRelease") == 0)
+    {
+        AbandonTests_Child_GracefulExit_CloseBeforeRelease();
+    }
     else if (test_strcmp(argv[2], "AbandonTests_Child_AbruptExit") == 0)
     {
         AbandonTests_Child_AbruptExit();
     }
+    else if (test_strcmp(argv[2], "AbandonTests_Child_TryLock") == 0)
+    {
+        AbandonTests_Child_TryLock();
+    }
     ExitProcess(PASS);
     return PASS;
 }