Merge lp:~jassmith/do/clear-universe into lp:do

[Jason Smith]

[Robert Dyer (psybers)]

On line 117 of the diff, use 'source' instead of 's' in the anon method.

Proof-read those 2 huge comment blocks for grammar (missing paren in one, 'so too will the old universe be' => no 'be').

[Robert Dyer (psybers)]

Don't we have a similar clearing problem with UniverseUpdateLoop()? If so, it needs a temporary universe too.

Then ReloadAction() and ReloadSource() don't need to have for loops checking if actions/items are in the universe because a (temporary) empty one will always be passed in.

review: Needs Information

[Jason Smith (jassmith)]

> Don't we have a similar clearing problem with UniverseUpdateLoop()? If so, it
> needs a temporary universe too.
>
> Then ReloadAction() and ReloadSource() don't need to have for loops checking
> if actions/items are in the universe because a (temporary) empty one will
> always be passed in.

We do not need to check in UniverseUpdateLoop because it will not happen there. And if it does it is self rectifying in a sense. Reload gets called after any plugin enable/disable so this is not a problem.

As for the other things, grammer and naming, will fix

[lp:~jassmith/do/clear-universe]

1256. By Jason Smith <jason@t500> on 2009-06-24

Clean up merge request items

[Alex Launi (alexlauni)]

review: Approve

=== modified file 'Do/src/Do.Core/UniverseManager.cs'
--- Do/src/Do.Core/UniverseManager.cs	2009-06-24 20:45:55 +0000
+++ Do/src/Do.Core/UniverseManager.cs	2009-06-24 20:46:47 +0000
@@ -28,6 +28,8 @@
 using Do.Universe;
 using Do.Universe.Safe;
 
+using UniverseCollection = System.Collections.Generic.Dictionary<string, Do.Universe.Item>;
+
 namespace Do.Core
 {
 	
@@ -35,8 +37,9 @@
 	{
 
 		Thread update_thread;
-		Dictionary<string, Item> universe;
+		UniverseCollection universe;
 		EventHandler initialized;
+		object universe_lock;
 		
 		const float epsilon = 0.00001f;
 		
@@ -75,7 +78,8 @@
 		
 		public UniverseManager ()
 		{
-			universe = new Dictionary<string, Item> ();
+			universe = new UniverseCollection ();
+			universe_lock = new object ();
 
 			update_thread = new Thread (new ThreadStart (UniverseUpdateLoop));
 			update_thread.IsBackground = true;
@@ -117,7 +121,7 @@
 		
 		public IEnumerable<Item> Search (string query, IEnumerable<Type> filter, Item other)
 		{
-			lock (universe) 
+			lock (universe_lock) 
 				return Search (query, filter, universe.Values, other);
 		}
 		
@@ -158,22 +162,24 @@
 		void UniverseUpdateLoop ()
 		{
 			Random rand = new Random ();
-			DateTime startUpdate = DateTime.Now;
+			DateTime startUpdate = DateTime.UtcNow;
 
 			while (true) {
 				Thread.Sleep (UpdateTimeout);
 				if (Do.Controller.IsSummoned) continue;
-				startUpdate = DateTime.Now;
+				startUpdate = DateTime.UtcNow;
 				
 				if (rand.Next (10) == 0) {
-					ReloadActions ();
+					ReloadActions (universe);
 				}
 				
 				foreach (ItemSource source in PluginManager.ItemSources) {
-					ReloadSource (source);
-					if (UpdateRunTime < DateTime.Now - startUpdate) {
+					ReloadSource (source, universe);
+					
+					if (UpdateRunTime < DateTime.UtcNow - startUpdate) {
 						Thread.Sleep (UpdateTimeout);
-						startUpdate = DateTime.Now;
+						// sleeping for a bit
+						startUpdate = DateTime.UtcNow;
 					}
 				}
 			}
@@ -182,10 +188,10 @@
 		/// <summary>
 		/// Reloads all actions in the universe.
 		/// </summary>
-		void ReloadActions ()
+		void ReloadActions (UniverseCollection universe)
 		{
 			Log<UniverseManager>.Debug ("Reloading actions...");
-			lock (universe) {
+			lock (universe_lock) {
 				foreach (Act action in PluginManager.Actions) {
 					if (universe.ContainsKey (action.UniqueId))
 						universe.Remove (action.UniqueId);
@@ -200,7 +206,7 @@
 		/// not be called on the main thread to avoid blocking the UI if the
 		/// item source takes a long time to update.
 		/// </summary>
-		void ReloadSource (ItemSource source)
+		void ReloadSource (ItemSource source, UniverseCollection universe)
 		{
 			SafeItemSource safeSource;
 			IEnumerable<Item> oldItems, newItems;
@@ -215,7 +221,7 @@
 			safeSource.UpdateItems ();
 			newItems = safeSource.Items;
 			
-			lock (universe) {
+			lock (universe_lock) {
 				foreach (Item item in oldItems) {
 					if (universe.ContainsKey (item.UniqueId))
 						universe.Remove (item.UniqueId);
@@ -229,8 +235,18 @@
 		void ReloadUniverse ()
 		{
 			Log<UniverseManager>.Info ("Reloading universe...");
-			ReloadActions ();
-			PluginManager.ItemSources.ForEach (ReloadSource);
+			
+			// A new temporary universe is created so that searches made during the reload (as threaded 
+			// searches are allowed will not see an interuption in available items. Additionally this 
+			// serves to clear out unused items that are orphaned from their item service.
+			UniverseCollection tmpUniverse = new UniverseCollection ();
+			ReloadActions (tmpUniverse);
+			PluginManager.ItemSources.ForEach (s => ReloadSource (s, tmpUniverse));
+			
+			// Clearing the old universe is not needed and considered harmful as enumerables in existence
+			// already will be based off the old universe. Clearing it may cause an exception to be thrown.
+			// Once those enumerables are destroyed, so too will the old universe be.
+			universe = tmpUniverse;
 			Log<UniverseManager>.Info ("Universe contains {0} items.", universe.Count);
 		}
 		
@@ -242,7 +258,7 @@
 		/// </param>
 		public void AddItems (IEnumerable<Item> items)
 		{
-			lock (universe) {
+			lock (universe_lock) {
 				foreach (Item item in items) {
 					if (universe.ContainsKey (item.UniqueId)) continue;
 					universe [item.UniqueId] = item;
@@ -259,7 +275,7 @@
 		/// </param>
 		public void DeleteItems (IEnumerable<Item> items)
 		{
-			lock (universe) {
+			lock (universe_lock) {
 				foreach (Item item in items) {
 					universe.Remove (item.UniqueId);
 				}
@@ -277,7 +293,7 @@
 		/// </param>
 		public bool TryGetItemForUniqueId (string uid, out Item element)
 		{
-			lock (universe) {
+			lock (universe_lock) {
 				if (universe.ContainsKey (uid)) {
 					element = universe [uid];
 				} else {