- 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!).

svn path=/trunk/; revision=24569

reactos/ntoskrnl/KrnlFun.c

@@ -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:
 //

reactos/ntoskrnl/include/internal/ob_x.h

@@ -6,6 +6,30 @@
 * PROGRAMMERS:     Alex Ionescu (alex.ionescu@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
 ObpReleaseCapturedAttributes(IN POBJECT_CREATE_INFORMATION ObjectCreateInfo)

reactos/ntoskrnl/ob/obhandle.c

@@ -93,6 +93,8 @@ ObpDecrementHandleCount(IN PVOID ObjectBody,
     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 @@ ObpDecrementHandleCount(IN PVOID ObjectBody,
             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 @@ ObpDecrementHandleCount(IN PVOID ObjectBody,
     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 @@ ObpIncrementHandleCount(IN PVOID Object,
             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 @@ ObpIncrementHandleCount(IN PVOID Object,
     /* Increase the handle count */
     InterlockedIncrement(&ObjectHeader->HandleCount);
 
+    /* Release the lock */
+    ObpLeaveObjectTypeMutex(ObjectType);
+
     /* FIXME: Use the Handle Database */
     ProcessHandleCount = 0;
 
@@ -361,7 +388,7 @@ ObpIncrementHandleCount(IN PVOID Object,
     }
 
     /* 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 @@ ObpIncrementUnnamedHandleCount(IN PVOID Object,
             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;
@@ -446,6 +476,9 @@ ObpIncrementUnnamedHandleCount(IN PVOID Object,
     /* Increase the handle count */
     InterlockedIncrement(&ObjectHeader->HandleCount);
 
+    /* Release the object type */
+    ObpLeaveObjectTypeMutex(ObjectType);
+
     /* FIXME: Use the Handle Database */
     ProcessHandleCount = 0;
 
@@ -461,7 +494,7 @@ ObpIncrementUnnamedHandleCount(IN PVOID Object,
     }
 
     /* 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 @@ NTAPI
 ObCloseHandle(IN HANDLE Handle,
               IN KPROCESSOR_MODE AccessMode)
 {
-    //
-    // Call the internal API
-    //
+    /* Call the internal API */
     return ObpCloseHandle(Handle, AccessMode);
 }
 
@@ -2142,7 +2173,7 @@ NtDuplicateObject(IN HANDLE SourceProcessHandle,
             hTarget,
             TargetProcessHandle,
             Status);
-    ObDereferenceObject(TargetProcess);
+    ObDereferenceObject(Target);
     ObDereferenceObject(SourceProcess);
     return Status;
 }

reactos/ntoskrnl/ob/oblife.c

@@ -151,6 +151,9 @@ ObpDeleteObject(IN PVOID Object)
     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)))
     {
@@ -158,6 +161,9 @@ ObpDeleteObject(IN PVOID Object)
         RemoveEntryList(&CreatorInfo->TypeList);
     }
 
+    /* Release the lock */
+    ObpLeaveObjectTypeMutex(ObjectType);
+
     /* Check if we have a name */
     if ((NameInfo) && (NameInfo->Name.Buffer))
     {
@@ -782,15 +788,14 @@ ObCreateObject(IN KPROCESSOR_MODE ProbeMode OPTIONAL,
             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 @@ ObCreateObjectType(IN PUNICODE_STRING TypeName,
     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,
@@ -1032,6 +1040,9 @@ ObCreateObjectType(IN PUNICODE_STRING TypeName,
         ObpObjectTypes[LocalObjectType->Index - 1] = LocalObjectType;
     }
 
+    /* Release the object type */
+    ObpLeaveObjectTypeMutex(LocalObjectType);
+
     /* Check if we're actually creating the directory object itself */
     if (!(ObpTypeDirectoryObject) ||
         (ObpInsertEntryDirectory(ObpTypeDirectoryObject, &Context, Header)))

reactos/ntoskrnl/ob/obname.c

@@ -191,6 +191,9 @@ ObpDeleteNameCheck(IN PVOID Object)
                                          &Context);
         if ((Object) && !(ObjectHeader->HandleCount))
         {
+            /* Lock the object type */
+            ObpEnterObjectTypeMutex(ObjectType);
+
             /* First delete it from the directory */
             ObpDeleteEntryDirectory(&Context);
 
@@ -210,6 +213,9 @@ ObpDeleteNameCheck(IN PVOID Object)
                                                        NULL);
             }
 
+            /* Release the lock */
+            ObpLeaveObjectTypeMutex(ObjectType);
+
             /* Free the name */
             ExFreePool(ObjectNameInfo->Name.Buffer);
             RtlInitEmptyUnicodeString(&ObjectNameInfo->Name, NULL, 0);