- Object Manager Improvement Patch 1/3:

- Re-implement delayed object deletion by using an optimized Object Reaper based on OBJECT_HEADER->NextToFree. Thanks to Thomas for the algorithm.
  - Refactor object deletion into two operations: Removal and de-allocation (this is needed for failure during allocation, which we don't do yet).
  - BugFixes:
    * After freeing an object header structure, also clear the pointer so we don't attempt it again. 
    * Clear the handle database if there is one
    * Make sure the create info flag is set before deleting captured attributes.
    * Use the allocation pool tag when de-allocating.
    * Use OBJECT_TYPE accounting for tracking the numbef of objects.
    * Remove the object from it's typelist if it has creator information.
    * Call the security procedure to delete the security descriptor.

svn path=/trunk/; revision=22098

reactos/include/ndk/obtypes.h

@@ -416,7 +416,7 @@ typedef struct _OBJECT_HEADER
     union
     {
         LONG HandleCount;
-        PVOID NextToFree;
+        volatile PVOID NextToFree;
     };
     POBJECT_TYPE Type;
     UCHAR NameInfoOffset;

reactos/ntoskrnl/include/internal/ob.h

@@ -30,6 +30,8 @@ extern POBJECT_TYPE ObTypeObjectType;
 extern POBJECT_DIRECTORY NameSpaceRoot;
 extern POBJECT_DIRECTORY ObpTypeDirectoryObject;
 extern PHANDLE_TABLE ObpKernelHandleTable;
+extern WORK_QUEUE_ITEM ObpReaperWorkItem;
+extern volatile PVOID ObpReaperList;
 
 BOOLEAN
 NTAPI
@@ -157,10 +159,16 @@ VOID
 STDCALL
 ObKillProcess(PEPROCESS Process);
 
-NTSTATUS
+VOID
-ObpDeleteObjectDpcLevel(
+FASTCALL
-    IN POBJECT_HEADER ObjectHeader,
+ObpDeleteObject(
-    IN LONG OldPointerCount
+    IN PVOID Object
+);
+
+VOID
+NTAPI
+ObpReapObject(
+    IN PVOID Unused
 );
 
 /* Security descriptor cache functions */

reactos/ntoskrnl/ob/oblife.c

@@ -21,53 +21,27 @@ extern ULONG NtGlobalFlag;
 
 POBJECT_TYPE ObTypeObjectType = NULL;
 KEVENT ObpDefaultObject;
-
+WORK_QUEUE_ITEM ObpReaperWorkItem;
-typedef struct _RETENTION_CHECK_PARAMS
+volatile PVOID ObpReaperList;
-{
-    WORK_QUEUE_ITEM WorkItem;
-    POBJECT_HEADER ObjectHeader;
-} RETENTION_CHECK_PARAMS, *PRETENTION_CHECK_PARAMS;
 
 /* PRIVATE FUNCTIONS *********************************************************/
 
-static NTSTATUS
+VOID
-ObpDeleteObject(POBJECT_HEADER Header)
+FASTCALL
+ObpDeallocateObject(IN PVOID Object)
 {
-    PVOID HeaderLocation = Header;
+    PVOID HeaderLocation;
+    POBJECT_HEADER Header;
+    POBJECT_TYPE ObjectType;
     POBJECT_HEADER_HANDLE_INFO HandleInfo;
     POBJECT_HEADER_NAME_INFO NameInfo;
     POBJECT_HEADER_CREATOR_INFO CreatorInfo;
+    PAGED_CODE();
 
-    DPRINT("ObpDeleteObject(Header %p)\n", Header);
+    /* Get the header and assume this is what we'll free */
-    if (KeGetCurrentIrql() != PASSIVE_LEVEL)
+    Header = OBJECT_TO_OBJECT_HEADER(Object);
-    {
+    ObjectType = Header->Type;
-        DPRINT("ObpDeleteObject called at an unsupported IRQL.  Use ObpDeleteObjectDpcLevel instead.\n");
+    HeaderLocation = Header;
-        KEBUGCHECK(0);
-    }
-
-    if (Header->Type != NULL &&
-        Header->Type->TypeInfo.DeleteProcedure != NULL)
-    {
-        Header->Type->TypeInfo.DeleteProcedure(&Header->Body);
-    }
-
-    if (Header->SecurityDescriptor != NULL)
-    {
-        ObpRemoveSecurityDescriptor(Header->SecurityDescriptor);
-    }
-
-    if (OBJECT_HEADER_TO_NAME_INFO(Header))
-    {
-        if(OBJECT_HEADER_TO_NAME_INFO(Header)->Name.Buffer)
-        {
-            ExFreePool(OBJECT_HEADER_TO_NAME_INFO(Header)->Name.Buffer);
-        }
-    }
-    if (Header->ObjectCreateInfo)
-    {
-        ObpReleaseCapturedAttributes(Header->ObjectCreateInfo);
-        ExFreePool(Header->ObjectCreateInfo);
-    }
 
     /* To find the header, walk backwards from how we allocated */
     if ((CreatorInfo = OBJECT_HEADER_TO_CREATOR_INFO(Header)))
@@ -83,88 +57,124 @@ ObpDeleteObject(POBJECT_HEADER Header)
         HeaderLocation = HandleInfo;
     }
 
-    DPRINT("ObPerformRetentionChecks() = Freeing object\n");
+    /* Check if we have create info */
-    ExFreePool(HeaderLocation);
+    if (Header->Flags & OB_FLAG_CREATE_INFO)
+    {
+        /* Double-check that it exists */
+        if (Header->ObjectCreateInfo)
+        {
+            /* Free it */
+            ObpReleaseCapturedAttributes(Header->ObjectCreateInfo);
+            Header->ObjectCreateInfo = NULL;
+        }
+    }
 
-    return(STATUS_SUCCESS);
+    /* Check if a handle database was active */
+    if ((HandleInfo) && (Header->Flags & OB_FLAG_SINGLE_PROCESS))
+    {
+        /* Free it */
+        ExFreePool(HandleInfo->HandleCountDatabase);
+        HandleInfo->HandleCountDatabase = NULL;
+    }
+
+    /* Check if we have a name */
+    if ((NameInfo) && (NameInfo->Name.Buffer))
+    {
+        /* Free it */
+        ExFreePool(NameInfo->Name.Buffer);
+        NameInfo->Name.Buffer = NULL;
+    }
+
+    /* Free the object using the same allocation tag */
+    ExFreePoolWithTag(HeaderLocation,
+                      ObjectType ? TAG('T', 'j', 'b', 'O') : ObjectType->Key);
+
+    /* Decrease the total */
+    ObjectType->TotalNumberOfObjects--;
 }
 
-
+VOID
-VOID STDCALL
+FASTCALL
-ObpDeleteObjectWorkRoutine (IN PVOID Parameter)
+ObpDeleteObject(IN PVOID Object)
 {
-    PRETENTION_CHECK_PARAMS Params = (PRETENTION_CHECK_PARAMS)Parameter;
+    POBJECT_HEADER Header;
-    /* ULONG Tag; */ /* See below */
+    POBJECT_TYPE ObjectType;
+    POBJECT_HEADER_NAME_INFO NameInfo;
+    POBJECT_HEADER_CREATOR_INFO CreatorInfo;
+    PAGED_CODE();
 
-    ASSERT(Params);
+    /* Get the header and type */
-    ASSERT(KeGetCurrentIrql() == PASSIVE_LEVEL); /* We need PAGED_CODE somewhere... */
+    Header = OBJECT_TO_OBJECT_HEADER(Object);
+    ObjectType = Header->Type;
 
-    /* Turn this on when we have ExFreePoolWithTag
+    /* Get creator and name information */
-    Tag = Params->ObjectHeader->Type->Tag; */
+    NameInfo = OBJECT_HEADER_TO_NAME_INFO(Header);
-    ObpDeleteObject(Params->ObjectHeader);
+    CreatorInfo = OBJECT_HEADER_TO_CREATOR_INFO(Header);
-    ExFreePool(Params);
+
-    /* ExFreePoolWithTag(Params, Tag); */
+    /* Check if the object is on a type list */
+    if ((CreatorInfo) && !(IsListEmpty(&CreatorInfo->TypeList)))
+    {
+        /* Remove the object from the type list */
+        RemoveEntryList(&CreatorInfo->TypeList);
+    }
+
+    /* Check if we have a name */
+    if ((NameInfo) && (NameInfo->Name.Buffer))
+    {
+        /* Free it */
+        ExFreePool(NameInfo->Name.Buffer);
+
+        /* Clean up the string so we don't try this again */
+        RtlInitUnicodeString(&NameInfo->Name, NULL);
+    }
+
+    /* Check if we have a security descriptor */
+    if (Header->SecurityDescriptor)
+    {
+        ObjectType->TypeInfo.SecurityProcedure(Object,
+                                               DeleteSecurityDescriptor,
+                                               0,
+                                               NULL,
+                                               NULL,
+                                               &Header->SecurityDescriptor,
+                                               0,
+                                               NULL);
+    }
+
+    /* Check if we have a delete procedure */
+    if (ObjectType->TypeInfo.DeleteProcedure)
+    {
+        /* Call it */
+        ObjectType->TypeInfo.DeleteProcedure(Object);
+    }
+
+    /* Now de-allocate all object members */
+    ObpDeallocateObject(Object);
 }
 
-
+VOID
-NTSTATUS
+NTAPI
-ObpDeleteObjectDpcLevel(IN POBJECT_HEADER ObjectHeader,
+ObpReapObject(IN PVOID Parameter)
-                        IN LONG OldPointerCount)
 {
-#if 0
+    POBJECT_HEADER ReapObject;
-    if (ObjectHeader->PointerCount < 0)
+    PVOID NextObject;
+
+    /* Start reaping */
+    while((ReapObject = InterlockedExchangePointer(&ObpReaperList, NULL)))
     {
-        CPRINT("Object %p/%p has invalid reference count (%d)\n",
+        /* Start deletion loop */
-            ObjectHeader, HEADER_TO_BODY(ObjectHeader),
+        do
-            ObjectHeader->PointerCount);
-        KEBUGCHECK(0);
-    }
-
-    if (ObjectHeader->HandleCount < 0)
         {
-        CPRINT("Object %p/%p has invalid handle count (%d)\n",
+            /* Get the next object */
-            ObjectHeader, HEADER_TO_BODY(ObjectHeader),
+            NextObject = ReapObject->NextToFree;
-            ObjectHeader->HandleCount);
+
-        KEBUGCHECK(0);
+            /* Delete the object */
+            ObpDeleteObject(&ReapObject->Body);
+
+            /* Move to the next one */
+            ReapObject = NextObject;
+        } while(NextObject != NULL);
     }
-#endif
-
-
-    switch (KeGetCurrentIrql ())
-    {
-    case PASSIVE_LEVEL:
-        return ObpDeleteObject (ObjectHeader);
-
-    case APC_LEVEL:
-    case DISPATCH_LEVEL:
-        {
-            PRETENTION_CHECK_PARAMS Params;
-
-            /*
-            We use must succeed pool here because if the allocation fails
-            then we leak memory.
-            */
-            Params = (PRETENTION_CHECK_PARAMS)
-                ExAllocatePoolWithTag(NonPagedPoolMustSucceed,
-                sizeof(RETENTION_CHECK_PARAMS),
-                ObjectHeader->Type->Key);
-            Params->ObjectHeader = ObjectHeader;
-            ExInitializeWorkItem(&Params->WorkItem,
-                ObpDeleteObjectWorkRoutine,
-                (PVOID)Params);
-            ExQueueWorkItem(&Params->WorkItem,
-                CriticalWorkQueue);
-        }
-        return STATUS_PENDING;
-
-    default:
-        DPRINT("ObpDeleteObjectDpcLevel called at unsupported "
-            "IRQL %u!\n", KeGetCurrentIrql());
-        KEBUGCHECK(0);
-        return STATUS_UNSUCCESSFUL;
-    }
-
-    return STATUS_SUCCESS;
 }
 
 NTSTATUS

reactos/ntoskrnl/ob/obref.c

@@ -104,28 +104,37 @@ FASTCALL
 ObfDereferenceObject(IN PVOID Object)
 {
     POBJECT_HEADER Header;
-    LONG NewPointerCount;
-    BOOL Permanent;
 
-    ASSERT(Object);
+    /* Extract the object header */
-
-    /* Extract the object header. */
     Header = OBJECT_TO_OBJECT_HEADER(Object);
-    Permanent = Header->Flags & OB_FLAG_PERMANENT;
-
-    /*
-    Drop our reference and get the new count so we can tell if this was the
-    last reference.
-    */
-    NewPointerCount = InterlockedDecrement(&Header->PointerCount);
-    DPRINT("ObfDereferenceObject(0x%x)==%d\n", Object, NewPointerCount);
-    ASSERT(NewPointerCount >= 0);
 
     /* Check whether the object can now be deleted. */
-    if (NewPointerCount == 0 &&
+    if (!(InterlockedDecrement(&Header->PointerCount)) &&
-        !Permanent)
+        !(Header->Flags & OB_FLAG_PERMANENT))
     {
-        ObpDeleteObjectDpcLevel(Header, NewPointerCount);
+        /* Sanity check */
+        ASSERT(!Header->HandleCount);
+
+        /* Check if we're at PASSIVE */
+        if (KeGetCurrentIrql() == PASSIVE_LEVEL)
+        {
+            /* Remove the object */
+            ObpDeleteObject(Object);
+        }
+        else
+        {
+            /* Add us to the list */
+            do
+            {
+                Header->NextToFree = ObpReaperList;
+            } while (InterlockedCompareExchangePointer(&ObpReaperList,
+                                                       Header,
+                                                       Header->NextToFree) !=
+                     Header->NextToFree);
+
+            /* Queue the work item */
+            ExQueueWorkItem(&ObpReaperWorkItem, DelayedWorkQueue);
+        }
     }
 }