[ros-dev] Re: [ros-svn] [ion] 13832: - FreeLdr Part II (ntoskrnl is now relocated, removes 3GB compiler flag). Note that there is a bug in LD which Filip and I are examining, so do not try this yet.

[Hartmut Birr]

Hi,

I can fix my problem by protecting the access to some entries of the
object header with a global spinnlock, but I doesn't like this. Has
anyone a better idea?

- Hartmut



Hartmut Birr schrieb:

>Alex Ionescu schrieb:
>
>  
>
>>James Tabor wrote:
>>
>>    
>>
>>>BugCheck Alert!
>>>
>>>
>>>      
>>>
>>Hmm...looks like after reverting Thomas's patch and using Hartmut's,
>>the problems are happening again.
>>So, it would seem that Thomas' patch is more correct? Or perhaps both
>>should be used?
>>T/H: Let me know!
>>
>>    
>>
>Hi,  
>
>I've written a little test program which does always crash ros with my
>patch on the smp machine (up not tested). The program does create 10
>threads which try to open a non existing registry key in an endless
>loop. It does not crash with Thomas' patch. His patch adds one reference
>to much to the first parsed/opened key. This makes it impossible to
>delete a key and to unload the registry at shut down. I think we have a
>problem outside from the registry. It seems there is a gap between the
>deleting of an object after the last dereference operation and the next
>create operation for a new object with the same name. One thread has
>reopend the object and use it and the other delete the same object. I
>see very often values of 0xcccccxxx on bug checks and the wrong
>reference value -858993460 is also 0xcccccccc.   
>
>- Hartmut
>
>  
>
>------------------------------------------------------------------------
>
>_______________________________________________
>Ros-dev mailing list
>Ros-dev at reactos.com
>http://reactos.com:8080/mailman/listinfo/ros-dev
>

-------------- next part --------------

M:\Sandbox\ros_work\reactos>set SVN_EDITOR=notepad 

M:\Sandbox\ros_work\reactos>d:\programme\subversion\bin\svn.exe diff ntoskrnl\ob\object.c        
Index: ntoskrnl/ob/object.c
===================================================================
--- ntoskrnl/ob/object.c	(revision 13857)
+++ ntoskrnl/ob/object.c	(working copy)
@@ -22,6 +22,7 @@
   POBJECT_HEADER ObjectHeader;
 } RETENTION_CHECK_PARAMS, *PRETENTION_CHECK_PARAMS;
 
+KSPIN_LOCK ObpLock = 0;
 
 /* FUNCTIONS ************************************************************/
 
@@ -818,6 +819,8 @@
 			   IN KPROCESSOR_MODE AccessMode)
 {
    POBJECT_HEADER Header;
+   KIRQL oldIrql;
+   NTSTATUS Status;
    
    /* NOTE: should be possible to reference an object above APC_LEVEL! */
 
@@ -849,22 +852,32 @@
 	DPRINT("eip %x\n", ((PULONG)&Object)[-1]);
      }
  
-   if (Header->CloseInProcess)
-   {
-      if (Header->ObjectType == PsProcessType)
-        {
-	  return STATUS_PROCESS_IS_TERMINATING;
-	}
-      if (Header->ObjectType == PsThreadType)
-        {
-	  return STATUS_THREAD_IS_TERMINATING;
-	}
-      return(STATUS_UNSUCCESSFUL);
-   }
+   KeAcquireSpinLock(&ObpLock, &oldIrql);
 
-   InterlockedIncrement(&Header->RefCount);
+   if (Header->CloseInProcess ||
+       (0 == Header->RefCount && 0 == Header->HandleCount && !Header->Permanent))
+     {
+       if (Header->ObjectType == PsProcessType)
+         {
+	   Status = STATUS_PROCESS_IS_TERMINATING;
+	 }
+       else if (Header->ObjectType == PsThreadType)
+         {
+	   Status = STATUS_THREAD_IS_TERMINATING;
+	 }
+       else
+         {
+           Status = STATUS_UNSUCCESSFUL;
+         }
+     }
+   else
+     {
+       InterlockedIncrement(&Header->RefCount);
+       Status = STATUS_SUCCESS;
+     } 
+   KeReleaseSpinLock(&ObpLock, oldIrql);
    
-   return(STATUS_SUCCESS);
+   return Status;
 }
 
 
@@ -960,7 +973,7 @@
 
 STATIC NTSTATUS
 ObpDeleteObjectDpcLevel(IN POBJECT_HEADER ObjectHeader,
-			IN LONG OldRefCount)
+			IN KIRQL oldIrql)
 {
   if (ObjectHeader->RefCount < 0)
     {
@@ -981,11 +994,14 @@
   if (ObjectHeader->CloseInProcess)
     {
       KEBUGCHECK(0);
+      KeReleaseSpinLock(&ObpLock, oldIrql);
       return STATUS_UNSUCCESSFUL;
     }
   ObjectHeader->CloseInProcess = TRUE;
   
-  switch (KeGetCurrentIrql ())
+  KeReleaseSpinLock(&ObpLock, oldIrql);
+  
+  switch (oldIrql)
     {
     case PASSIVE_LEVEL:
       return ObpDeleteObject (ObjectHeader);
@@ -1043,18 +1059,23 @@
 ObfReferenceObject(IN PVOID Object)
 {
   POBJECT_HEADER Header;
+  KIRQL oldIrql;
 
   ASSERT(Object);
 
   Header = BODY_TO_HEADER(Object);
 
+  KeAcquireSpinLock(&ObpLock, &oldIrql);
+
   /* No one should be referencing an object once we are deleting it. */
-  if (Header->CloseInProcess)
+  if (Header->CloseInProcess || 
+      (0 == Header->RefCount && 0 == Header->HandleCount && !Header->Permanent))
     {
       KEBUGCHECK(0);
     }
 
   (VOID)InterlockedIncrement(&Header->RefCount);
+  KeReleaseSpinLock(&ObpLock, oldIrql);
 }
 
 
@@ -1081,9 +1102,12 @@
   LONG NewRefCount;
   BOOL Permanent;
   ULONG HandleCount;
+  KIRQL oldIrql;
 
   ASSERT(Object);
 
+  KeAcquireSpinLock(&ObpLock, &oldIrql);
+
   /* Extract the object header. */
   Header = BODY_TO_HEADER(Object);
   Permanent = Header->Permanent;
@@ -1101,8 +1125,12 @@
       HandleCount == 0 &&
       !Permanent)
     {
-      ObpDeleteObjectDpcLevel(Header, NewRefCount);
+      ObpDeleteObjectDpcLevel(Header, oldIrql);
     }
+  else
+    {
+      KeReleaseSpinLock(&ObpLock, oldIrql);
+    }
 }