Re: [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(a)reactos.com
> http://reactos.com:8080/mailman/listinfo/ros-dev
>  


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);
+    }
 }