[ros-diffs] [hbirr] 14017: Added a keep-alive reference to each key object.

Added a keep-alive reference to each key object.  
Lock the registry while accessing sub keys of a key object.  
Implemented a worker thread which removes all unused key objects.  
Fixed a bug which shows keys twice if a key is already opened.
Modified: trunk/reactos/ntoskrnl/cm/cm.h
Modified: trunk/reactos/ntoskrnl/cm/ntfunc.c
Modified: trunk/reactos/ntoskrnl/cm/registry.c
Modified: trunk/reactos/ntoskrnl/cm/regobj.c
  _____  

Modified: trunk/reactos/ntoskrnl/cm/cm.h
--- trunk/reactos/ntoskrnl/cm/cm.h	2005-03-13 17:01:59 UTC (rev
14016)
+++ trunk/reactos/ntoskrnl/cm/cm.h	2005-03-13 17:03:42 UTC (rev
14017)
@@ -353,6 +353,12 @@

 
   /* List of subkeys loaded */
   struct _KEY_OBJECT **SubKeys;
+
+  /* List entry into the global key object list */
+  LIST_ENTRY ListEntry;
+  
+  /* Time stamp for the last access by the parse routine */
+  ULONG TimeStamp;
 } KEY_OBJECT, *PKEY_OBJECT;
 
 /* Bits 31-22 (top 10 bits) of the cell index is the directory index */
@@ -529,10 +535,11 @@
 NTSTATUS
 CmiRemoveKeyFromList(IN PKEY_OBJECT NewKey);
 
-PKEY_OBJECT
+NTSTATUS
 CmiScanKeyList(IN PKEY_OBJECT Parent,
 	       IN PUNICODE_STRING KeyName,
-	       IN ULONG Attributes);
+	       IN ULONG Attributes,
+	       PKEY_OBJECT* ReturnedObject);
 
 NTSTATUS
 CmiCreateVolatileHive(PREGISTRY_HIVE *RegistryHive);
  _____  

Modified: trunk/reactos/ntoskrnl/cm/ntfunc.c
--- trunk/reactos/ntoskrnl/cm/ntfunc.c	2005-03-13 17:01:59 UTC (rev
14016)
+++ trunk/reactos/ntoskrnl/cm/ntfunc.c	2005-03-13 17:03:42 UTC (rev
14017)
@@ -21,6 +21,7 @@

 
 extern POBJECT_TYPE  CmiKeyType;
 extern PREGISTRY_HIVE  CmiVolatileHive;
+extern LIST_ENTRY CmiKeyObjectListHead;
 
 static BOOLEAN CmiRegistryInitialized = FALSE;
 
@@ -300,6 +301,8 @@
   KeEnterCriticalRegion();
   ExAcquireResourceExclusiveLite(&CmiRegistryLock, TRUE);
 
+  InsertTailList(&CmiKeyObjectListHead, &KeyObject->ListEntry);
+
   /* add key to subkeys of parent if needed */
   Status = CmiAddSubKey(KeyObject->RegistryHive,
 			KeyObject->ParentKey,
@@ -355,7 +358,7 @@
   ExReleaseResourceLite(&CmiRegistryLock);
   KeLeaveCriticalRegion();
 
-  ObDereferenceObject(KeyObject);
+  
   ObDereferenceObject(Object);
 
   if (Disposition)
@@ -419,6 +422,9 @@
 
   /* Dereference the object */
   ObDereferenceObject(KeyObject);
+  /* Remove the keep-alive reference */
+  ObDereferenceObject(KeyObject);
+
   if (KeyObject->RegistryHive != KeyObject->ParentKey->RegistryHive)
     ObDereferenceObject(KeyObject);
 
@@ -513,8 +519,7 @@
       for (i = 0; i < KeyObject->NumberOfSubKeys; i++)
 	{
 	  CurKey = KeyObject->SubKeys[i];
-	  if (CurKey->RegistryHive == CmiVolatileHive ||
-	      CurKey->RegistryHive != RegistryHive)
+	  if (CurKey->RegistryHive != RegistryHive)
 	    {
 	      if (j == Index)
 		break;
  _____  

Modified: trunk/reactos/ntoskrnl/cm/registry.c
--- trunk/reactos/ntoskrnl/cm/registry.c	2005-03-13 17:01:59 UTC
(rev 14016)
+++ trunk/reactos/ntoskrnl/cm/registry.c	2005-03-13 17:03:42 UTC
(rev 14017)
@@ -20,12 +20,15 @@

 
 POBJECT_TYPE  CmiKeyType = NULL;
 PREGISTRY_HIVE  CmiVolatileHive = NULL;
-KSPIN_LOCK  CmiKeyListLock;
 
 LIST_ENTRY CmiHiveListHead;
 
 ERESOURCE CmiRegistryLock;
 
+KTIMER CmiWorkerTimer;
+LIST_ENTRY CmiKeyObjectListHead;
+ULONG CmiTimer = 0;
+
 volatile BOOLEAN CmiHiveSyncEnabled = FALSE;
 volatile BOOLEAN CmiHiveSyncPending = FALSE;
 KDPC CmiHiveSyncDpc;
@@ -241,6 +244,67 @@
   CmiCheckByName(Verbose, L"User");
 }
 
+VOID STDCALL
+CmiWorkerThread(PVOID Param)
+{
+  NTSTATUS Status;
+  PLIST_ENTRY CurrentEntry;
+  PKEY_OBJECT CurrentKey;
+  ULONG Count;
+
+
+  while (1)
+  {
+    Status = KeWaitForSingleObject(&CmiWorkerTimer, 
+	                           Executive, 
+				   KernelMode, 
+				   FALSE, 
+				   NULL);
+    if (Status == STATUS_SUCCESS)
+    {
+      DPRINT("CmiWorkerThread\n");
+
+      /* Acquire hive lock */
+      KeEnterCriticalRegion();
+      ExAcquireResourceExclusiveLite(&CmiRegistryLock, TRUE);
+
+      CmiTimer++;
+
+      Count = 0;
+      CurrentEntry = CmiKeyObjectListHead.Blink;
+      while (CurrentEntry != &CmiKeyObjectListHead)
+      {
+         CurrentKey = CONTAINING_RECORD(CurrentEntry, KEY_OBJECT,
ListEntry);
+	 if (CurrentKey->TimeStamp + 120 > CmiTimer)
+	 {
+	    /* The object was accessed in the last 10min */
+	    break;
+	 }
+	 if (1 == ObGetObjectPointerCount(CurrentKey) &&
+	     !(CurrentKey->Flags & KO_MARKED_FOR_DELETE))
+	 {
+	    ObDereferenceObject(CurrentKey);
+            CurrentEntry = CmiKeyObjectListHead.Blink;
+	    Count++;
+	 }
+	 else
+	 {
+	    CurrentEntry = CurrentEntry->Blink;
+	 }
+      }
+      ExReleaseResourceLite(&CmiRegistryLock);
+      KeLeaveCriticalRegion();
+
+      DPRINT("Removed %d key objects\n", Count);
+
+    }
+    else
+    {
+      KEBUGCHECK(0);
+    }
+  }
+}
+
 VOID
 INIT_FUNCTION
 STDCALL
@@ -279,6 +343,9 @@
   HANDLE RootKeyHandle;
   HANDLE KeyHandle;
   NTSTATUS Status;
+  LARGE_INTEGER DueTime;
+  HANDLE ThreadHandle;
+  CLIENT_ID ThreadId;
 
   /*  Initialize the Key object type  */
   CmiKeyType = ExAllocatePool(NonPagedPool, sizeof(OBJECT_TYPE));
@@ -311,6 +378,29 @@
   /* Initialize registry lock */
   ExInitializeResourceLite(&CmiRegistryLock);
 
+  /* Initialize the key object list */
+  InitializeListHead(&CmiKeyObjectListHead);
+
+  /* Initialize the worker timer */
+  KeInitializeTimerEx(&CmiWorkerTimer, SynchronizationTimer);
+
+  /* Initialize the worker thread */
+  Status = PsCreateSystemThread(&ThreadHandle,
+				THREAD_ALL_ACCESS,
+				NULL,
+				NULL,
+				&ThreadId,
+				CmiWorkerThread,
+				NULL);
+  if (!NT_SUCCESS(Status))
+  {
+    KEBUGCHECK(0);
+  }
+
+  /* Start the timer */  
+  DueTime.QuadPart = -1;
+  KeSetTimerEx(&CmiWorkerTimer, DueTime, 5000, NULL); /* 5sec */
+
   /*  Build volatile registry store  */
   Status = CmiCreateVolatileHive (&CmiVolatileHive);
   ASSERT(NT_SUCCESS(Status));
@@ -346,6 +436,7 @@
   RootKey->NumberOfSubKeys = 0;
   RootKey->SubKeys = NULL;
   RootKey->SizeOfSubKeys = 0;
+  InsertTailList(&CmiKeyObjectListHead, &RootKey->ListEntry);
   Status = RtlpCreateUnicodeString(&RootKey->Name, L"Registry",
NonPagedPool);
   ASSERT(NT_SUCCESS(Status));
 
@@ -672,6 +763,7 @@
   NewKey->KeyCell = CmiGetCell (RegistryHive, NewKey->KeyCellOffset,
NULL);
   NewKey->Flags = 0;
   NewKey->NumberOfSubKeys = 0;
+  InsertTailList(&CmiKeyObjectListHead, &NewKey->ListEntry);
   if (NewKey->KeyCell->NumberOfSubKeys != 0)
     {
       NewKey->SubKeys = ExAllocatePool(NonPagedPool,
@@ -726,6 +818,8 @@
   PREGISTRY_HIVE Hive;
   HANDLE KeyHandle;
   NTSTATUS Status;
+  PLIST_ENTRY CurrentEntry;
+  PKEY_OBJECT CurrentKey;
 
   DPRINT("CmiDisconnectHive() called\n");
 
@@ -765,11 +859,36 @@
       return STATUS_INVALID_PARAMETER;
     }
 
+  /* Acquire registry lock exclusively */
+  KeEnterCriticalRegion();
+  ExAcquireResourceExclusiveLite(&CmiRegistryLock, TRUE);
+
+  CurrentEntry = CmiKeyObjectListHead.Flink;
+  while (CurrentEntry != &CmiKeyObjectListHead)
+  {
+     CurrentKey = CONTAINING_RECORD(CurrentEntry, KEY_OBJECT,
ListEntry);
+     if (1 == ObGetObjectPointerCount(CurrentKey) &&
+	 !(CurrentKey->Flags & KO_MARKED_FOR_DELETE))
+     {
+        ObDereferenceObject(CurrentKey);
+        CurrentEntry = CmiKeyObjectListHead.Flink;
+     }
+     else
+     {
+        CurrentEntry = CurrentEntry->Flink;
+     }
+  }
+
   if (ObGetObjectHandleCount (KeyObject) != 0 ||
       ObGetObjectPointerCount (KeyObject) != 2)
     {
-      DPRINT1 ("Hive is still in use\n");
+      DPRINT1 ("Hive is still in use (hc %d, rc %d)\n",
ObGetObjectHandleCount (KeyObject), ObGetObjectPointerCount
(KeyObject));
       ObDereferenceObject (KeyObject);
+      
+      /* Release registry lock */
+      ExReleaseResourceLite (&CmiRegistryLock);
+      KeLeaveCriticalRegion();
+
       return STATUS_UNSUCCESSFUL;
     }
 
@@ -781,6 +900,10 @@
 
   *RegistryHive = Hive;
 
+  /* Release registry lock */
+  ExReleaseResourceLite (&CmiRegistryLock);
+  KeLeaveCriticalRegion();
+
   DPRINT ("CmiDisconnectHive() done\n");
 
   return STATUS_SUCCESS;
  _____  

Modified: trunk/reactos/ntoskrnl/cm/regobj.c
--- trunk/reactos/ntoskrnl/cm/regobj.c	2005-03-13 17:01:59 UTC (rev
14016)
+++ trunk/reactos/ntoskrnl/cm/regobj.c	2005-03-13 17:03:42 UTC (rev
14017)
@@ -14,6 +14,8 @@

 
 #include "cm.h"
 
+extern LIST_ENTRY CmiKeyObjectListHead;
+extern ULONG CmiTimer;
 
 static NTSTATUS
 CmiGetLinkTarget(PREGISTRY_HIVE RegistryHive,
@@ -76,10 +78,22 @@
 		KeyName.Length);
   KeyName.Buffer[KeyName.Length / sizeof(WCHAR)] = 0;
 
+  /* Acquire hive lock */
+  KeEnterCriticalRegion();
+  ExAcquireResourceExclusiveLite(&CmiRegistryLock, TRUE);
 
-  FoundObject = CmiScanKeyList(ParsedKey,
-			       &KeyName,
-			       Attributes);
+
+  Status = CmiScanKeyList(ParsedKey,
+			  &KeyName,
+			  Attributes,
+			  &FoundObject);
+  if (!NT_SUCCESS(Status))
+  {
+     ExReleaseResourceLite(&CmiRegistryLock);
+     KeLeaveCriticalRegion();
+     RtlFreeUnicodeString(&KeyName);
+     return Status;
+  }
   if (FoundObject == NULL)
     {
       Status = CmiScanForSubKey(ParsedKey->RegistryHive,
@@ -91,6 +105,8 @@
 				Attributes);
       if (!NT_SUCCESS(Status) || (SubKeyCell == NULL))
 	{
+          ExReleaseResourceLite(&CmiRegistryLock);
+          KeLeaveCriticalRegion();
 	  RtlFreeUnicodeString(&KeyName);
 	  return(STATUS_UNSUCCESSFUL);
 	}
@@ -104,6 +120,9 @@
 				    &LinkPath);
 	  if (NT_SUCCESS(Status))
 	    {
+              ExReleaseResourceLite(&CmiRegistryLock);
+              KeLeaveCriticalRegion();
+
 	      DPRINT("LinkPath '%wZ'\n", &LinkPath);
 
 	      /* build new FullPath for reparsing */
@@ -140,7 +159,7 @@
 	}
 
       /* Create new key object and put into linked list */
-      DPRINT("CmiObjectParse: %s\n", Path);
+      DPRINT("CmiObjectParse: %S\n", *Path);
       Status = ObCreateObject(KernelMode,
 			      CmiKeyType,
 			      NULL,
@@ -152,14 +171,19 @@
 			      (PVOID*)&FoundObject);
       if (!NT_SUCCESS(Status))
 	{
+          ExReleaseResourceLite(&CmiRegistryLock);
+          KeLeaveCriticalRegion();
 	  RtlFreeUnicodeString(&KeyName);
 	  return(Status);
 	}
+      /* Add the keep-alive reference */
+      ObReferenceObject(FoundObject);
 
       FoundObject->Flags = 0;
       FoundObject->KeyCell = SubKeyCell;
       FoundObject->KeyCellOffset = BlockOffset;
       FoundObject->RegistryHive = ParsedKey->RegistryHive;
+      InsertTailList(&CmiKeyObjectListHead, &FoundObject->ListEntry);
       RtlpCreateUnicodeString(&FoundObject->Name,
               KeyName.Buffer, NonPagedPool);
       CmiAddKeyToList(ParsedKey, FoundObject);
@@ -179,7 +203,12 @@
 	  if (NT_SUCCESS(Status))
 	    {
 	      DPRINT("LinkPath '%wZ'\n", &LinkPath);
+	
+              ExReleaseResourceLite(&CmiRegistryLock);
+              KeLeaveCriticalRegion();
 
+	      ObDereferenceObject(FoundObject);
+
 	      /* build new FullPath for reparsing */
 	      TargetPath.MaximumLength = LinkPath.MaximumLength;
 	      if (EndPtr != NULL)
@@ -212,13 +241,15 @@
 	      return(STATUS_REPARSE);
 	    }
 	}
-
-      ObReferenceObjectByPointer(FoundObject,
-				 STANDARD_RIGHTS_REQUIRED,
-				 NULL,
-				 UserMode);
     }
 
+  RemoveEntryList(&FoundObject->ListEntry);
+  InsertHeadList(&CmiKeyObjectListHead, &FoundObject->ListEntry);
+  FoundObject->TimeStamp = CmiTimer;
+
+  ExReleaseResourceLite(&CmiRegistryLock);
+  KeLeaveCriticalRegion();
+
   DPRINT("CmiObjectParse: %s\n", FoundObject->Name);
 
   *Path = EndPtr;
@@ -274,11 +305,16 @@
 
   ObReferenceObject (ParentKeyObject);
 
+  /* Acquire hive lock */
+  KeEnterCriticalRegion();
+  ExAcquireResourceExclusiveLite(&CmiRegistryLock, TRUE);
+
   if (!NT_SUCCESS(CmiRemoveKeyFromList(KeyObject)))
     {
       DPRINT1("Key not found in parent list ???\n");
     }
 
+  RemoveEntryList(&KeyObject->ListEntry);
   RtlFreeUnicodeString(&KeyObject->Name);
 
   if (KeyObject->Flags & KO_MARKED_FOR_DELETE)
@@ -302,6 +338,9 @@
 
   ObDereferenceObject (ParentKeyObject);
 
+  ExReleaseResourceLite(&CmiRegistryLock);
+  KeLeaveCriticalRegion();
+
   if (KeyObject->NumberOfSubKeys)
     {
       KEBUGCHECK(REGISTRY_ERROR);
@@ -532,11 +571,9 @@
 CmiAddKeyToList(PKEY_OBJECT ParentKey,
 		PKEY_OBJECT NewKey)
 {
-  KIRQL OldIrql;
 
   DPRINT("ParentKey %.08x\n", ParentKey);
 
-  KeAcquireSpinLock(&CmiKeyListLock, &OldIrql);
 
   if (ParentKey->SizeOfSubKeys <= ParentKey->NumberOfSubKeys)
     {
@@ -568,7 +605,6 @@
 		NULL,
 		UserMode);
   NewKey->ParentKey = ParentKey;
-  KeReleaseSpinLock(&CmiKeyListLock, OldIrql);
 }
 
 
@@ -576,11 +612,9 @@
 CmiRemoveKeyFromList(PKEY_OBJECT KeyToRemove)
 {
   PKEY_OBJECT ParentKey;
-  KIRQL OldIrql;
   DWORD Index;
 
   ParentKey = KeyToRemove->ParentKey;
-  KeAcquireSpinLock(&CmiKeyListLock, &OldIrql);
   /* FIXME: If list maintained in alphabetic order, use dichotomic
search */
   for (Index = 0; Index < ParentKey->NumberOfSubKeys; Index++)
     {
@@ -591,7 +625,6 @@
 			  &ParentKey->SubKeys[Index + 1],
 			  (ParentKey->NumberOfSubKeys - Index - 1) *
sizeof(PKEY_OBJECT));
 	  ParentKey->NumberOfSubKeys--;
-	  KeReleaseSpinLock(&CmiKeyListLock, OldIrql);
 
 	  DPRINT("Dereference parent key: 0x%x\n", ParentKey);
 	
@@ -599,25 +632,23 @@
 	  return STATUS_SUCCESS;
 	}
     }
-  KeReleaseSpinLock(&CmiKeyListLock, OldIrql);
 
   return STATUS_UNSUCCESSFUL;
 }
 
 
-PKEY_OBJECT
+NTSTATUS
 CmiScanKeyList(PKEY_OBJECT Parent,
 	       PUNICODE_STRING KeyName,
-	       ULONG Attributes)
+	       ULONG Attributes,
+	       PKEY_OBJECT* ReturnedObject)
 {
   PKEY_OBJECT CurKey;
-  KIRQL OldIrql;
   ULONG Index;
-
+  
   DPRINT("Scanning key list for: %wZ (Parent: %wZ)\n",
 	 KeyName, &Parent->Name);
 
-  KeAcquireSpinLock(&CmiKeyListLock, &OldIrql);
   /* FIXME: if list maintained in alphabetic order, use dichotomic
search */
   for (Index=0; Index < Parent->NumberOfSubKeys; Index++)
     {
@@ -627,8 +658,7 @@
 	  if ((KeyName->Length == CurKey->Name.Length)
 	      && (_wcsicmp(KeyName->Buffer, CurKey->Name.Buffer) == 0))
 	    {
-	      KeReleaseSpinLock(&CmiKeyListLock, OldIrql);
-	      return CurKey;
+	      break;
 	    }
 	}
       else
@@ -636,14 +666,26 @@
 	  if ((KeyName->Length == CurKey->Name.Length)
 	      && (wcscmp(KeyName->Buffer, CurKey->Name.Buffer) == 0))
 	    {
-	      KeReleaseSpinLock(&CmiKeyListLock, OldIrql);
-	      return CurKey;
+	      break;
 	    }
 	}
     }
-  KeReleaseSpinLock(&CmiKeyListLock, OldIrql);
-
-  return NULL;
+  
+  if (Index < Parent->NumberOfSubKeys)
+  {
+     if (CurKey->Flags & KO_MARKED_FOR_DELETE)
+     {
+        *ReturnedObject = NULL;
+	return STATUS_UNSUCCESSFUL;
+     }
+     ObReferenceObject(CurKey);
+     *ReturnedObject = CurKey;
+  }
+  else
+  {
+     *ReturnedObject = NULL;
+  }
+  return STATUS_SUCCESS;
 }