[ros-diffs] [ion] 24569: - Implement Object Type Resource Lock, since object types are shared across all objects and thread-safety is critical. Used it everywhere where I think it's needed. Thomas, can you check if I missed anything please? - Use interlocked increase/decrease for accounting variables inside the Object Type instead of acquiring a full lock or not being thread safe. - Clear the creator type list of an object if it lost all its handles. - Fix a bug in NtduplicateObject which was potentially derefernecing a garbage pointer (thanks Prefast!).

ion at svn.reactos.org ion at svn.reactos.org
Thu Oct 19 04:54:49 CEST 2006 

Author: ion
Date: Thu Oct 19 06:54:48 2006
New Revision: 24569

URL: http://svn.reactos.org/svn/reactos?rev=24569&view=rev
Log:
- Implement Object Type Resource Lock, since object types are shared across all objects and thread-safety is critical. Used it everywhere where I think it's needed. Thomas, can you check if I missed anything please?
- Use interlocked increase/decrease for accounting variables inside the Object Type instead of acquiring a full lock or not being thread safe.
- Clear the creator type list of an object if it lost all its handles.
- Fix a bug in NtduplicateObject which was potentially derefernecing a garbage pointer (thanks Prefast!).

Modified:
    trunk/reactos/ntoskrnl/KrnlFun.c
    trunk/reactos/ntoskrnl/include/internal/ob_x.h
    trunk/reactos/ntoskrnl/ob/obhandle.c
    trunk/reactos/ntoskrnl/ob/oblife.c
    trunk/reactos/ntoskrnl/ob/obname.c

Modified: trunk/reactos/ntoskrnl/KrnlFun.c
URL: http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/KrnlFun.c?rev=24569&r1=24568&r2=24569&view=diff
==============================================================================
--- trunk/reactos/ntoskrnl/KrnlFun.c (original)
+++ trunk/reactos/ntoskrnl/KrnlFun.c Thu Oct 19 06:54:48 2006
@@ -22,7 +22,6 @@
 // Ob:
 //  - Fix bug related to Deferred Loading (don't requeue active work item).
 //  - Add Directory Lock.
-//  - Use Object Type Mutex/Lock.
 //
 // Ke:
 //

Modified: trunk/reactos/ntoskrnl/include/internal/ob_x.h
URL: http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/include/internal/ob_x.h?rev=24569&r1=24568&r2=24569&view=diff
==============================================================================
--- trunk/reactos/ntoskrnl/include/internal/ob_x.h (original)
+++ trunk/reactos/ntoskrnl/include/internal/ob_x.h Thu Oct 19 06:54:48 2006
@@ -5,6 +5,30 @@
 * PURPOSE:         Intenral Inlined Functions for the Object Manager
 * PROGRAMMERS:     Alex Ionescu (alex.ionescu at reactos.org)
 */
+
+VOID
+FORCEINLINE
+ObpEnterObjectTypeMutex(IN POBJECT_TYPE ObjectType)
+{
+    /* Sanity check */
+    ASSERT(KeGetCurrentIrql() <= APC_LEVEL);
+
+    /* Enter a critical region and acquire the resource */
+    KeEnterCriticalRegion();
+    ExAcquireResourceExclusiveLite(&ObjectType->Mutex, TRUE);
+}
+
+VOID
+FORCEINLINE
+ObpLeaveObjectTypeMutex(IN POBJECT_TYPE ObjectType)
+{
+    /* Enter a critical region and acquire the resource */
+    ExReleaseResourceLite(&ObjectType->Mutex);
+    KeLeaveCriticalRegion();
+
+    /* Sanity check */
+    ASSERT(KeGetCurrentIrql() <= APC_LEVEL);
+}
 
 VOID
 FORCEINLINE

Modified: trunk/reactos/ntoskrnl/ob/obhandle.c
URL: http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/ob/obhandle.c?rev=24569&r1=24568&r2=24569&view=diff
==============================================================================
--- trunk/reactos/ntoskrnl/ob/obhandle.c (original)
+++ trunk/reactos/ntoskrnl/ob/obhandle.c Thu Oct 19 06:54:48 2006
@@ -93,6 +93,8 @@
     POBJECT_HEADER ObjectHeader;
     POBJECT_TYPE ObjectType;
     LONG SystemHandleCount, ProcessHandleCount;
+    LONG NewCount;
+    POBJECT_HEADER_CREATOR_INFO CreatorInfo;
 
     /* Get the object type and header */
     ObjectHeader = OBJECT_TO_OBJECT_HEADER(ObjectBody);
@@ -104,12 +106,31 @@
             ObjectHeader->HandleCount,
             ObjectHeader->PointerCount);
 
+    /* Lock the object type */
+    ObpEnterObjectTypeMutex(ObjectType);
+
     /* FIXME: The process handle count should be in the Handle DB. Investigate */
     SystemHandleCount = ObjectHeader->HandleCount;
     ProcessHandleCount = 0;
 
     /* Decrement the handle count */
-    InterlockedDecrement(&ObjectHeader->HandleCount);
+    NewCount = InterlockedDecrement(&ObjectHeader->HandleCount);
+
+    /* Check if we're out of handles */
+    if (!NewCount)
+    {
+        /* Get the creator info */
+        CreatorInfo = OBJECT_HEADER_TO_CREATOR_INFO(ObjectHeader);
+        if ((CreatorInfo) && !(IsListEmpty(&CreatorInfo->TypeList)))
+        {
+            /* Remove it from the list and re-initialize it */
+            RemoveEntryList(&CreatorInfo->TypeList);
+            InitializeListHead(&CreatorInfo->TypeList);
+        }
+    }
+
+    /* Release the lock */
+    ObpLeaveObjectTypeMutex(ObjectType);
 
     /* Check if we have a close procedure */
     if (ObjectType->TypeInfo.CloseProcedure)
@@ -126,7 +147,7 @@
     ObpDeleteNameCheck(ObjectBody);
 
     /* Decrease the total number of handles for this type */
-    ObjectType->TotalNumberOfHandles--;
+    InterlockedDecrement(&ObjectType->TotalNumberOfHandles);
     OBTRACE(OB_HANDLE_DEBUG,
             "%s - Decremented count for: %p. HC LC %lx %lx\n",
             __FUNCTION__,
@@ -306,6 +327,9 @@
             ObjectHeader->HandleCount,
             ObjectHeader->PointerCount);
 
+    /* Lock the object type */
+    ObpEnterObjectTypeMutex(ObjectType);
+
     /* Charge quota and remove the creator info flag */
     Status = ObpChargeQuotaForObject(ObjectHeader, ObjectType);
     if (!NT_SUCCESS(Status)) return Status;
@@ -346,6 +370,9 @@
     /* Increase the handle count */
     InterlockedIncrement(&ObjectHeader->HandleCount);
 
+    /* Release the lock */
+    ObpLeaveObjectTypeMutex(ObjectType);
+
     /* FIXME: Use the Handle Database */
     ProcessHandleCount = 0;
 
@@ -361,7 +388,7 @@
     }
 
     /* Increase total number of handles */
-    ObjectType->TotalNumberOfHandles++;
+    InterlockedIncrement(&ObjectType->TotalNumberOfHandles);
     OBTRACE(OB_HANDLE_DEBUG,
             "%s - Incremented count for: %p. Reason: %lx HC LC %lx %lx\n",
             __FUNCTION__,
@@ -423,6 +450,9 @@
             ObjectHeader->HandleCount,
             ObjectHeader->PointerCount);
 
+    /* Lock the object type */
+    ObpEnterObjectTypeMutex(ObjectType);
+
     /* Charge quota and remove the creator info flag */
     Status = ObpChargeQuotaForObject(ObjectHeader, ObjectType);
     if (!NT_SUCCESS(Status)) return Status;
@@ -445,6 +475,9 @@
 
     /* Increase the handle count */
     InterlockedIncrement(&ObjectHeader->HandleCount);
+
+    /* Release the object type */
+    ObpLeaveObjectTypeMutex(ObjectType);
 
     /* FIXME: Use the Handle Database */
     ProcessHandleCount = 0;
@@ -461,7 +494,7 @@
     }
 
     /* Increase total number of handles */
-    ObjectType->TotalNumberOfHandles++;
+    InterlockedIncrement(&ObjectType->TotalNumberOfHandles);
     OBTRACE(OB_HANDLE_DEBUG,
             "%s - Incremented count for: %p. UNNAMED HC LC %lx %lx\n",
             __FUNCTION__,
@@ -2000,9 +2033,7 @@
 ObCloseHandle(IN HANDLE Handle,
               IN KPROCESSOR_MODE AccessMode)
 {
-    //
-    // Call the internal API
-    //
+    /* Call the internal API */
     return ObpCloseHandle(Handle, AccessMode);
 }
 
@@ -2142,7 +2173,7 @@
             hTarget,
             TargetProcessHandle,
             Status);
-    ObDereferenceObject(TargetProcess);
+    ObDereferenceObject(Target);
     ObDereferenceObject(SourceProcess);
     return Status;
 }

Modified: trunk/reactos/ntoskrnl/ob/oblife.c
URL: http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/ob/oblife.c?rev=24569&r1=24568&r2=24569&view=diff
==============================================================================
--- trunk/reactos/ntoskrnl/ob/oblife.c (original)
+++ trunk/reactos/ntoskrnl/ob/oblife.c Thu Oct 19 06:54:48 2006
@@ -151,12 +151,18 @@
     NameInfo = OBJECT_HEADER_TO_NAME_INFO(Header);
     CreatorInfo = OBJECT_HEADER_TO_CREATOR_INFO(Header);
 
+    /* Lock the object type */
+    ObpEnterObjectTypeMutex(ObjectType);
+
     /* Check if the object is on a type list */
     if ((CreatorInfo) && !(IsListEmpty(&CreatorInfo->TypeList)))
     {
         /* Remove the object from the type list */
         RemoveEntryList(&CreatorInfo->TypeList);
     }
+
+    /* Release the lock */
+    ObpLeaveObjectTypeMutex(ObjectType);
 
     /* Check if we have a name */
     if ((NameInfo) && (NameInfo->Name.Buffer))
@@ -782,15 +788,14 @@
             if (!PagedPoolCharge)
             {
                 /* Save it */
-                PagedPoolCharge = ObjectType->TypeInfo.DefaultPagedPoolCharge;
+                PagedPoolCharge = Type->TypeInfo.DefaultPagedPoolCharge;
             }
 
             /* Check for nonpaged charge */
             if (!NonPagedPoolCharge)
             {
                 /* Save it */
-                NonPagedPoolCharge = ObjectType->
-                                     TypeInfo.DefaultNonPagedPoolCharge;
+                NonPagedPoolCharge = Type->TypeInfo.DefaultNonPagedPoolCharge;
             }
 
             /* Write the pool charges */
@@ -1019,6 +1024,9 @@
     ExInitializeResourceLite(&LocalObjectType->Mutex);
     InitializeListHead(&LocalObjectType->TypeList);
 
+    /* Lock the object type */
+    ObpEnterObjectTypeMutex(LocalObjectType);
+
     /* Get creator info and insert it into the type list */
     CreatorInfo = OBJECT_HEADER_TO_CREATOR_INFO(Header);
     if (CreatorInfo) InsertTailList(&ObTypeObjectType->TypeList,
@@ -1031,6 +1039,9 @@
         /* It fits, insert it */
         ObpObjectTypes[LocalObjectType->Index - 1] = LocalObjectType;
     }
+
+    /* Release the object type */
+    ObpLeaveObjectTypeMutex(LocalObjectType);
 
     /* Check if we're actually creating the directory object itself */
     if (!(ObpTypeDirectoryObject) ||

Modified: trunk/reactos/ntoskrnl/ob/obname.c
URL: http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/ob/obname.c?rev=24569&r1=24568&r2=24569&view=diff
==============================================================================
--- trunk/reactos/ntoskrnl/ob/obname.c (original)
+++ trunk/reactos/ntoskrnl/ob/obname.c Thu Oct 19 06:54:48 2006
@@ -191,6 +191,9 @@
                                          &Context);
         if ((Object) && !(ObjectHeader->HandleCount))
         {
+            /* Lock the object type */
+            ObpEnterObjectTypeMutex(ObjectType);
+
             /* First delete it from the directory */
             ObpDeleteEntryDirectory(&Context);
 
@@ -209,6 +212,9 @@
                                                        TypeInfo.PoolType,
                                                        NULL);
             }
+
+            /* Release the lock */
+            ObpLeaveObjectTypeMutex(ObjectType);
 
             /* Free the name */
             ExFreePool(ObjectNameInfo->Name.Buffer);

More information about the Ros-diffs mailing list