Jacek Caban : ntdll: Don' t modify LockCount in RtlLeaveCriticalSection if section is not acquired.

Alexandre Julliard julliard at winehq.org
Tue Mar 7 15:43:59 CST 2017


Module: wine
Branch: master
Commit: 14206495e301e0cb8e1b1d878a2baf7470daf96b
URL:    http://source.winehq.org/git/wine.git/?a=commit;h=14206495e301e0cb8e1b1d878a2baf7470daf96b

Author: Jacek Caban <jacek at codeweavers.com>
Date:   Mon Mar  6 12:51:22 2017 +0100

ntdll: Don't modify LockCount in RtlLeaveCriticalSection if section is not acquired.

Signed-off-by: Jacek Caban <jacek at codeweavers.com>
Signed-off-by: Sebastian Lackner <sebastian at fds-team.de>
Signed-off-by: Alexandre Julliard <julliard at winehq.org>

---

 dlls/ntdll/critsection.c |  6 ++++-
 dlls/ntdll/tests/rtl.c   | 60 ++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 65 insertions(+), 1 deletion(-)

diff --git a/dlls/ntdll/critsection.c b/dlls/ntdll/critsection.c
index e2eb364..e405b08 100644
--- a/dlls/ntdll/critsection.c
+++ b/dlls/ntdll/critsection.c
@@ -665,7 +665,11 @@ BOOL WINAPI RtlIsCriticalSectionLockedByThread( RTL_CRITICAL_SECTION *crit )
  */
 NTSTATUS WINAPI RtlLeaveCriticalSection( RTL_CRITICAL_SECTION *crit )
 {
-    if (--crit->RecursionCount) interlocked_dec( &crit->LockCount );
+    if (--crit->RecursionCount)
+    {
+        if (crit->RecursionCount > 0) interlocked_dec( &crit->LockCount );
+        else ERR( "section %p is not acquired\n", crit );
+    }
     else
     {
         crit->OwningThread = 0;
diff --git a/dlls/ntdll/tests/rtl.c b/dlls/ntdll/tests/rtl.c
index 43c26bd..b18cccf 100644
--- a/dlls/ntdll/tests/rtl.c
+++ b/dlls/ntdll/tests/rtl.c
@@ -2094,6 +2094,65 @@ static void test_RtlInitializeCriticalSectionEx(void)
     RtlDeleteCriticalSection(&cs);
 }
 
+static void test_RtlLeaveCriticalSection(void)
+{
+    RTL_CRITICAL_SECTION cs;
+    NTSTATUS status;
+
+    if (!pRtlInitializeCriticalSectionEx)
+        return; /* Skip winxp */
+
+    status = RtlInitializeCriticalSection(&cs);
+    ok(!status, "RtlInitializeCriticalSection failed: %x\n", status);
+
+    status = RtlEnterCriticalSection(&cs);
+    ok(!status, "RtlEnterCriticalSection failed: %x\n", status);
+    todo_wine
+    ok(cs.LockCount == -2, "expected LockCount == -2, got %d\n", cs.LockCount);
+    ok(cs.RecursionCount == 1, "expected RecursionCount == 1, got %d\n", cs.RecursionCount);
+    ok(cs.OwningThread == ULongToHandle(GetCurrentThreadId()), "unexpected OwningThread\n");
+
+    status = RtlLeaveCriticalSection(&cs);
+    ok(!status, "RtlLeaveCriticalSection failed: %x\n", status);
+    ok(cs.LockCount == -1, "expected LockCount == -1, got %d\n", cs.LockCount);
+    ok(cs.RecursionCount == 0, "expected RecursionCount == 0, got %d\n", cs.RecursionCount);
+    ok(!cs.OwningThread, "unexpected OwningThread %p\n", cs.OwningThread);
+
+    /*
+     * Trying to leave a section that wasn't acquired modifies RecusionCount to an invalid value,
+     * but doesn't modify LockCount so that an attempt to enter the section later will work.
+     */
+    status = RtlLeaveCriticalSection(&cs);
+    ok(!status, "RtlLeaveCriticalSection failed: %x\n", status);
+    ok(cs.LockCount == -1, "expected LockCount == -1, got %d\n", cs.LockCount);
+    ok(cs.RecursionCount == -1, "expected RecursionCount == -1, got %d\n", cs.RecursionCount);
+    ok(!cs.OwningThread, "unexpected OwningThread %p\n", cs.OwningThread);
+
+    /* and again */
+    status = RtlLeaveCriticalSection(&cs);
+    ok(!status, "RtlLeaveCriticalSection failed: %x\n", status);
+    ok(cs.LockCount == -1, "expected LockCount == -1, got %d\n", cs.LockCount);
+    ok(cs.RecursionCount == -2, "expected RecursionCount == -2, got %d\n", cs.RecursionCount);
+    ok(!cs.OwningThread, "unexpected OwningThread %p\n", cs.OwningThread);
+
+    /* entering section fixes RecursionCount */
+    status = RtlEnterCriticalSection(&cs);
+    ok(!status, "RtlEnterCriticalSection failed: %x\n", status);
+    todo_wine
+    ok(cs.LockCount == -2, "expected LockCount == -2, got %d\n", cs.LockCount);
+    ok(cs.RecursionCount == 1, "expected RecursionCount == 1, got %d\n", cs.RecursionCount);
+    ok(cs.OwningThread == ULongToHandle(GetCurrentThreadId()), "unexpected OwningThread\n");
+
+    status = RtlLeaveCriticalSection(&cs);
+    ok(!status, "RtlLeaveCriticalSection failed: %x\n", status);
+    ok(cs.LockCount == -1, "expected LockCount == -1, got %d\n", cs.LockCount);
+    ok(cs.RecursionCount == 0, "expected RecursionCount == 0, got %d\n", cs.RecursionCount);
+    ok(!cs.OwningThread, "unexpected OwningThread %p\n", cs.OwningThread);
+
+    status = RtlDeleteCriticalSection(&cs);
+    ok(!status, "RtlDeleteCriticalSection failed: %x\n", status);
+}
+
 START_TEST(rtl)
 {
     InitFunctionPtrs();
@@ -2125,4 +2184,5 @@ START_TEST(rtl)
     test_RtlDecompressBuffer();
     test_RtlIsCriticalSectionLocked();
     test_RtlInitializeCriticalSectionEx();
+    test_RtlLeaveCriticalSection();
 }




More information about the wine-cvs mailing list