Enchant.Net - handle dictionaries being disposed of explicitly:

dictionary adds Disposed event
broker removes dictionary from cache when it is disposed
broker holds list of weak references in cache so dictionaries can still be garbage collected automatically when they go out of scope


git-svn-id: svn+ssh://svn.abisource.com/svnroot/enchant/trunk@24226 bcba8976-2d24-0410-9c9c-aab3bd5fdfd6

4 files changed, 359 insertions, 169 deletions

diff --git a/src/bindings/Enchant.Net/Broker.cs b/src/bindings/Enchant.Net/Broker.cs
index 9a28948..e7a53d2 100644
--- a/src/bindings/Enchant.Net/Broker.cs
+++ b/src/bindings/Enchant.Net/Broker.cs
@@ -30,8 +30,8 @@ namespace Enchant
 		private IList<DictionaryInfo> _dictionaries;

 		private bool _disposed = false;

 		private IList<ProviderInfo> _providers;

-	    private Dictionary<string, Dictionary> _dictionaryCache;

-	    private Dictionary<string, Dictionary> _pwlDictionaryCache;

+	    private readonly Dictionary<string, WeakReference> _dictionaryCache;

+	    private readonly Dictionary<string, WeakReference> _pwlDictionaryCache;

 	    private bool _cacheDictionaries = true;

 

 		public Broker()

@@ -42,8 +42,8 @@ namespace Enchant
 			{

 				throw new ApplicationException("Unable to initialize broker");

 			}

-            _dictionaryCache = new Dictionary<string, Dictionary>();

-            _pwlDictionaryCache = new Dictionary<string, Dictionary>();

+            _dictionaryCache = new Dictionary<string, WeakReference>();

+            _pwlDictionaryCache = new Dictionary<string, WeakReference>();

 		}

 

 		public IEnumerable<ProviderInfo> Providers

@@ -59,157 +59,222 @@ namespace Enchant
 			}

 		}

 

-    private void InitializeProviderList()

-    {

-      _providers = new List<ProviderInfo>();

-      Bindings.enchant_broker_describe(_handle,

-                                       delegate(ProviderInfo provider)

-                                       {

-                                         _providers.Add(provider);

-                                       });

-      VerifyNoErrors();

-    }

-

-    public IEnumerable<DictionaryInfo> Dictionaries

-		{

-			get

-			{

-				VerifyNotDisposed();

-				if (_dictionaries == null)

-				{

-					InitializeDictionaryList();

-				}

-				return _dictionaries;

-			}

-		}

+        private void InitializeProviderList()

+        {

+          _providers = new List<ProviderInfo>();

+          Bindings.enchant_broker_describe(_handle,

+                                           delegate(ProviderInfo provider)

+                                           {

+                                             _providers.Add(provider);

+                                           });

+          VerifyNoErrors();

+        }

 

-	    public bool CacheDictionaries

-	    {

-	        get { return this._cacheDictionaries; }

-	        set { this._cacheDictionaries = value; }

-	    }

+        public IEnumerable<DictionaryInfo> Dictionaries

+		    {

+			    get

+			    {

+				    VerifyNotDisposed();

+				    if (_dictionaries == null)

+				    {

+					    InitializeDictionaryList();

+				    }

+				    return _dictionaries;

+			    }

+		    }

+

+        public bool CacheDictionaries

+        {

+            get { return this._cacheDictionaries; }

+            set { this._cacheDictionaries = value; }

+        }

 

 	    private void InitializeDictionaryList()

-    {

-      _dictionaries = new List<DictionaryInfo>();

-      Bindings.enchant_broker_list_dicts(_handle,

-                                         delegate(DictionaryInfo dictionary)

-                                         {

-                                           _dictionaries.Add(dictionary);

-                                         });

-      VerifyNoErrors();

-    }

-

-		#region IDisposable Members

-

-    public void Dispose()

-		{

-			Dispose(true);

-			GC.SuppressFinalize(this);

-		}

+        {

+          _dictionaries = new List<DictionaryInfo>();

+          Bindings.enchant_broker_list_dicts(_handle,

+                                             delegate(DictionaryInfo dictionary)

+                                             {

+                                               _dictionaries.Add(dictionary);

+                                             });

+          VerifyNoErrors();

+        }

+

+	    #region IDisposable Members

 

-		#endregion

+        public void Dispose()

+		    {

+			    Dispose(true);

+			    GC.SuppressFinalize(this);

+		    }

 

-    private void Dispose(bool disposing)

-    {

-      if (!_disposed)

-      {

-        if (disposing)

+		    #endregion

+

+        private void Dispose(bool disposing)

         {

-          // dispose-only, i.e. non-finalizable logic

-          _handle.Dispose();

+          if (!_disposed)

+          {

+            if (disposing)

+            {

+                // dispose-only, i.e. non-finalizable logic

+                _handle.Dispose();

+                DisposeAllDictionariesFromCache(_dictionaryCache);

+                DisposeAllDictionariesFromCache(_pwlDictionaryCache);

+            }

+

+            // shared (dispose and finalizable) cleanup logic

+            _disposed = true;

+          }

         }

 

-        // shared (dispose and finalizable) cleanup logic

-        _disposed = true;

-      }

-    }

+	    private static void DisposeAllDictionariesFromCache(ICollection<KeyValuePair<string, WeakReference>> cache) {

+	        List<Dictionary> dictionariesToDispose = new List<Dictionary>();

+            foreach (KeyValuePair<string, WeakReference> pair in cache)

+	        {

+	            if(pair.Value.IsAlive)

+	            {

+	                dictionariesToDispose.Add((Dictionary) pair.Value.Target);

+	            }

+	        }

+            cache.Clear();

+	        foreach (Dictionary dictionary in dictionariesToDispose)

+	        {

+                dictionary.Dispose();

+	        }

+	    }

 

-    private void VerifyNotDisposed()

-    {

-      if (_disposed)

-      {

-        throw new ObjectDisposedException("Dictionary");

-      }

-    }

+	    private void VerifyNotDisposed()

+        {

+          if (_disposed)

+          {

+            throw new ObjectDisposedException("Dictionary");

+          }

+        }

 

-		public Dictionary RequestDictionary(string language_tag)

-		{

-			VerifyNotDisposed();

-			if (language_tag == null)

-			{

-				throw new ArgumentNullException("language_tag");

-			}

-		    Dictionary dictionary;

-		    if(CacheDictionaries)

+	    public Dictionary RequestDictionary(string language_tag)

+	    {

+		    VerifyNotDisposed();

+		    if (language_tag == null)

+		    {

+			    throw new ArgumentNullException("language_tag");

+		    }

+            Dictionary dictionary = GetDictionaryFromCache(this._dictionaryCache, language_tag);

+            if(dictionary != null)

             {

-                if (_dictionaryCache.TryGetValue(language_tag, out dictionary))

-                {

-                    return dictionary;

-                }

+                return dictionary;

             }

-			SafeDictionaryHandle handle = Bindings.enchant_broker_request_dict(_handle, language_tag);

-			VerifyNoErrors();

-			if (handle.IsInvalid)

-			{

-				throw new ApplicationException("There is no provider that supplies a dictionary for " + language_tag);

-			}

-		    dictionary = new Dictionary(handle);

-            if(CacheDictionaries)

+	        return CreateDictionary(language_tag);

+	    }

+

+	    private Dictionary CreateDictionary(string language_tag) {

+	        SafeDictionaryHandle handle = Bindings.enchant_broker_request_dict(this._handle, language_tag);

+	        VerifyNoErrors();

+	        if (handle.IsInvalid)

+	        {

+	            throw new ApplicationException("There is no provider that supplies a dictionary for " + language_tag);

+	        }

+

+	        return CreateAndRegisterDictionary(handle, this._dictionaryCache, language_tag);

+	    }

+

+	    private Dictionary GetDictionaryFromCache(IDictionary<string, WeakReference> cache, string language_tag) {

+	        if(CacheDictionaries)

+	        {

+	            WeakReference dictionaryReference;

+	            if (cache.TryGetValue(language_tag, out dictionaryReference))

+	            {

+                    if (dictionaryReference.IsAlive)

+                    {

+                        return (Dictionary) dictionaryReference.Target;

+                    }

+	            }

+	        }

+	        return null;

+	    }

+

+	    private Dictionary CreateAndRegisterDictionary(SafeDictionaryHandle handle, IDictionary<string, WeakReference> cache, string language_tag) {

+	        Dictionary dictionary;

+	        dictionary = new Dictionary(handle);

+	        dictionary.Disposed += OnDictionaryDisposed;

+	        // always store the dictionaries we have created

+	        // so that we can dispose of them cleanly and give a 

+	        // better error message (ObjectDisposed) instead of a crash

+	        // if someone tries to use a dictionary after the broker

+	        // that created it has been disposed.

+	        cache[language_tag] = new WeakReference(dictionary);

+	        return dictionary;

+	    }

+

+	    private void OnDictionaryDisposed(object sender, EventArgs e)

+	    {

+	        Dictionary dictionary = (Dictionary) sender;

+

+            // try to remove from _dictionaryCache

+            if (!RemoveDictionaryFromCache(this._dictionaryCache, dictionary))

             {

-                _dictionaryCache[language_tag] = dictionary;

+                // try to remove from _pwlDictionaryCache

+                RemoveDictionaryFromCache(this._pwlDictionaryCache, dictionary);

             }

-		    return dictionary;

-		}

+	    }

 

-    public bool DictionaryExists(string language_tag)

-    {

-      VerifyNotDisposed();

-      if (language_tag == null)

-      {

-        throw new ArgumentNullException("language_tag");

-      }

-      int result = Bindings.enchant_broker_dict_exists(_handle, language_tag);

-      VerifyNoErrors();

-      if (result != 0 && result != 1)

-      {

-        throw new NotImplementedException(

-          "enchant_broker_dict_exists returned unexpected value that is currently unhandled.");

-      }

-      return result == 1;

-    }

-

-    public Dictionary RequestPwlDictionary(string pwlFile)

+	    private static bool RemoveDictionaryFromCache(IDictionary<string, WeakReference> cache, Dictionary dictionary) {

+	        foreach (KeyValuePair<string, WeakReference> pair in cache)

+	        {

+                if (pair.Value.IsAlive 

+                    && pair.Value.Target == dictionary)

+                {

+                    cache.Remove(pair.Key);

+                    return true;

+                }

+	        }

+	        return false;

+	    }

+

+        public bool DictionaryExists(string language_tag)

+        {

+          VerifyNotDisposed();

+          if (language_tag == null)

+          {

+            throw new ArgumentNullException("language_tag");

+          }

+          int result = Bindings.enchant_broker_dict_exists(_handle, language_tag);

+          VerifyNoErrors();

+          if (result != 0 && result != 1)

+          {

+            throw new NotImplementedException(

+              "enchant_broker_dict_exists returned unexpected value that is currently unhandled.");

+          }

+          return result == 1;

+        }

+

+        public Dictionary RequestPwlDictionary(string pwlFile)

 		{

 			VerifyNotDisposed();

 			if (pwlFile == null)

 			{

 				throw new ArgumentNullException("pwlFile");

 			}

-            Dictionary dictionary;

-            if (CacheDictionaries)

-            {

-                if (_dictionaryCache.TryGetValue(pwlFile, out dictionary))

-                {

-                    return dictionary;

-                }

-            }

 

-			SafeDictionaryHandle handle = Bindings.enchant_broker_request_pwl_dict(_handle, pwlFile);

-			VerifyNoErrors();

-			if (handle.IsInvalid)

-			{

-				throw new ApplicationException("Unable to create pwl file " + pwlFile);

-			}

-            dictionary = new Dictionary(handle);

-            if (CacheDictionaries)

+            Dictionary dictionary = GetDictionaryFromCache(this._pwlDictionaryCache, pwlFile);

+            if (dictionary != null)

             {

-                _dictionaryCache[pwlFile] = dictionary;

+                return dictionary;

             }

-            return dictionary;

+

+			return CreatePwlDictionary(pwlFile);

 		}

 

-		public void SetOrdering(string language_tag, string ordering)

+	    private Dictionary CreatePwlDictionary(string pwlFile) {

+	        SafeDictionaryHandle handle = Bindings.enchant_broker_request_pwl_dict(this._handle, pwlFile);

+	        VerifyNoErrors();

+	        if (handle.IsInvalid)

+	        {

+	            throw new ApplicationException("Unable to create pwl file " + pwlFile);

+	        }

+	        return CreateAndRegisterDictionary(handle, this._pwlDictionaryCache, pwlFile);

+	    }

+

+	    public void SetOrdering(string language_tag, string ordering)

 		{

 			VerifyNotDisposed();

 			Bindings.enchant_broker_set_ordering(_handle, language_tag, ordering);

diff --git a/src/bindings/Enchant.Net/Dictionary.cs b/src/bindings/Enchant.Net/Dictionary.cs
index c0d0f6e..e219210 100644
--- a/src/bindings/Enchant.Net/Dictionary.cs
+++ b/src/bindings/Enchant.Net/Dictionary.cs
@@ -87,6 +87,11 @@ namespace Enchant
 

         // shared (dispose and finalizable) cleanup logic

         _disposed = true;

+        if (disposing)

+        {

+            OnDisposed(); // call it here so will throw if someone uses

+            // it because _disposed has been set to true;

+        }

       }

     }

 

@@ -198,5 +203,15 @@ namespace Enchant
 			}

 			Bindings.enchant_dict_store_replacement(_handle, misspelling, correction);

 		}

+

+        /// <summary>

+        /// Occurs when a dictionary is disposed by a call to the Dispose method

+        /// </summary>

+        public event EventHandler Disposed = delegate { };

+

+        private void OnDisposed()

+        {

+            Disposed.Invoke(this, new EventArgs());

+        }

 	}

 }
\ No newline at end of file
diff --git a/unittests/Enchant.Net.Tests/BrokerTests.cs b/unittests/Enchant.Net.Tests/BrokerTests.cs
index 892df34..288fb69 100644
--- a/unittests/Enchant.Net.Tests/BrokerTests.cs
+++ b/unittests/Enchant.Net.Tests/BrokerTests.cs
@@ -81,28 +81,33 @@ namespace Enchant.Tests
 								Path.Combine(providerDir, "libenchant_ispell.dll"), true);

 			File.Copy("libenchant_myspell.dll",

 								Path.Combine(providerDir, "libenchant_myspell.dll"), true);

+            InstallDictionary("myspell", new string[] { "en_US.aff", "en_US.dic" });

+		}

 

-			string dictionarySourceDir =

-				Path.Combine(Path.Combine(Path.Combine(Path.Combine(Path.Combine(Path.Combine(

-																																					 Directory.GetCurrentDirectory(), ".."), ".."),

-																														"lib"), "share"),

-																	"enchant"), "myspell");

+        static private void InstallDictionary(string provider, IEnumerable<string> files)

+        {

+            string dictionarySourceDir =

+                    Path.Combine(Path.Combine(Path.Combine(Path.Combine(Path.Combine(Path.Combine(

+                                             Directory.GetCurrentDirectory(), ".."), ".."),

+                                             "lib"), "share"),

+                                              "enchant"), provider);

 

-			string dictionaryDestDir = Path.Combine(Path.Combine(Path.Combine(

-																														 Directory.GetCurrentDirectory(), "share"), "enchant"),

-																							"myspell");

+            string dictionaryDestDir = Path.Combine(Path.Combine(Path.Combine(

+                                                    Directory.GetCurrentDirectory(), "share"), "enchant"),

+                                                    provider);

 

-			if (!Directory.Exists(dictionaryDestDir))

-			{

-				Directory.CreateDirectory(dictionaryDestDir);

-			}

+            if (!Directory.Exists(dictionaryDestDir))

+            {

+                Directory.CreateDirectory(dictionaryDestDir);

+            }

 

-			File.Copy(Path.Combine(dictionarySourceDir, "en_US.aff"),

-								Path.Combine(dictionaryDestDir, "en_US.aff"), true);

+            foreach (string file in files)

+            {

+                File.Copy(Path.Combine(dictionarySourceDir, file),

+                          Path.Combine(dictionaryDestDir, file), true);

 

-			File.Copy(Path.Combine(dictionarySourceDir, "en_US.dic"),

-								Path.Combine(dictionaryDestDir, "en_US.dic"), true);

-		}

+            }

+        }

 

 		[TestFixtureTearDown]

 		public void FixtureTearDown()

@@ -187,7 +192,75 @@ namespace Enchant.Tests
 			}

 		}

 

-		[Test]

+        [Test]

+        public void RequestDictionary_CachingEnabled_DictionaryReRequested_SameReference()

+        {

+            using (Broker broker = new Broker())

+            {

+                broker.CacheDictionaries = true;

+                Dictionary dictionaryFirstRequest = broker.RequestDictionary("en_US");

+                Dictionary dictionarySecondRequest = broker.RequestDictionary("en_US");

+

+                Assert.AreSame(dictionaryFirstRequest, dictionarySecondRequest);

+            }

+        }

+

+        [Test]

+        public void RequestDictionary_CachingEnabled_DictionaryDisposedThenReRequested_DifferentReference()

+        {

+            using (Broker broker = new Broker())

+            {

+                broker.CacheDictionaries = true;

+                Dictionary dictionaryFirstRequest;

+                using (dictionaryFirstRequest = broker.RequestDictionary("en_US")) {}

+                Dictionary dictionarySecondRequest = broker.RequestDictionary("en_US");

+

+                Assert.AreNotSame(dictionaryFirstRequest, dictionarySecondRequest);

+            }

+        }

+

+        [Test]

+        public void RequestDictionary_CachingDisabled_DictionaryReRequested_DifferentReference()

+        {

+            using (Broker broker = new Broker())

+            {

+                broker.CacheDictionaries = false;

+                Dictionary dictionaryFirstRequest = broker.RequestDictionary("en_US");

+                Dictionary dictionarySecondRequest = broker.RequestDictionary("en_US");

+

+                Assert.AreNotSame(dictionaryFirstRequest, dictionarySecondRequest);

+            }

+        }

+

+        [Test]

+        public void RequestDictionary_CachingDisabled_DictionaryDisposedThenReRequested_DifferentReference()

+        {

+            using (Broker broker = new Broker())

+            {

+                broker.CacheDictionaries = false;

+                Dictionary dictionaryFirstRequest;

+                using (dictionaryFirstRequest = broker.RequestDictionary("en_US"))

+                {

+                }

+                Dictionary dictionarySecondRequest = broker.RequestDictionary("en_US");

+

+                Assert.AreNotSame(dictionaryFirstRequest, dictionarySecondRequest);

+            }

+        }

+

+        [Test]

+        [ExpectedException(typeof(ObjectDisposedException))]

+        public void Dispose_UseDictionaryAfterBrokerDisposed_Throws()

+        {

+            Dictionary dictionary;

+            using (Broker broker = new Broker())

+            {

+                dictionary = broker.RequestDictionary("en_US");

+            }

+            DictionaryInfo info = dictionary.Information;

+        }

+

+        [Test]

 		[ExpectedException(typeof (ApplicationException))]

 		public void RequestDictionary_DictionaryDoesNotExist_Throws()

 		{

@@ -219,5 +292,26 @@ namespace Enchant.Tests
 				broker.SetOrdering("en_US", "aspell, myspell, ispell");

 			}

 		}

+

+        [Test]

+        public void Finalize_DictionaryGoesOutOfScope_Finalized()

+        {

+            using (Broker broker = new Broker())

+            {

+                broker.CacheDictionaries = true;

+                WeakReference dictionaryReference = GetDictionaryReference(broker);

+                GC.Collect();

+                GC.WaitForPendingFinalizers();

+                GC.Collect();

+                Assert.IsFalse(dictionaryReference.IsAlive);

+            }

+        }

+

+        //this will allow the dictionary object to go out of scope

+	    private static WeakReference GetDictionaryReference(Broker broker) 

+        {

+            Dictionary dictionary = broker.RequestDictionary("en_US");

+            return new WeakReference(dictionary);

+        }

 	}

 }

diff --git a/unittests/Enchant.Net.Tests/DictionaryTests.cs b/unittests/Enchant.Net.Tests/DictionaryTests.cs
index 3b7850d..9f7bc39 100644
--- a/unittests/Enchant.Net.Tests/DictionaryTests.cs
+++ b/unittests/Enchant.Net.Tests/DictionaryTests.cs
@@ -22,6 +22,7 @@
 using System;

 using System.Collections.Generic;

 using System.IO;

+using System.Threading;

 using Microsoft.Win32;

 using NUnit.Framework;

 

@@ -78,7 +79,7 @@ namespace Enchant.Tests
 		public void FixtureSetup()

 		{

 			string providerDir = Path.Combine(Path.Combine(

-																					Directory.GetCurrentDirectory(), "lib"), "enchant");

+								 Directory.GetCurrentDirectory(), "lib"), "enchant");

 			if (!Directory.Exists(providerDir))

 			{

 				Directory.CreateDirectory(providerDir);

@@ -87,29 +88,34 @@ namespace Enchant.Tests
 								Path.Combine(providerDir, "libenchant_ispell.dll"), true);

 			File.Copy("libenchant_myspell.dll",

 								Path.Combine(providerDir, "libenchant_myspell.dll"), true);

-

-			string dictionarySourceDir =

-				Path.Combine(Path.Combine(Path.Combine(Path.Combine(Path.Combine(Path.Combine(

-																																					 Directory.GetCurrentDirectory(), ".."), ".."),

-																														"lib"), "share"),

-																	"enchant"), "myspell");

-

-			string dictionaryDestDir = Path.Combine(Path.Combine(Path.Combine(

-																														 Directory.GetCurrentDirectory(), "share"), "enchant"),

-																							"myspell");

-

-			if (!Directory.Exists(dictionaryDestDir))

-			{

-				Directory.CreateDirectory(dictionaryDestDir);

-			}

-

-			File.Copy(Path.Combine(dictionarySourceDir, "en_US.aff"),

-								Path.Combine(dictionaryDestDir, "en_US.aff"), true);

-

-			File.Copy(Path.Combine(dictionarySourceDir, "en_US.dic"),

-								Path.Combine(dictionaryDestDir, "en_US.dic"), true);

+            InstallDictionary("myspell", new string[]{"en_US.aff", "en_US.dic"});

 		}

 

+        static private void InstallDictionary(string provider, IEnumerable<string> files)

+        {

+            string dictionarySourceDir =

+                    Path.Combine(Path.Combine(Path.Combine(Path.Combine(Path.Combine(Path.Combine(

+                                             Directory.GetCurrentDirectory(), ".."), ".."),

+                                             "lib"), "share"),

+                                              "enchant"), provider);

+

+            string dictionaryDestDir = Path.Combine(Path.Combine(Path.Combine(

+                                                    Directory.GetCurrentDirectory(), "share"), "enchant"),

+                                                    provider);

+

+            if (!Directory.Exists(dictionaryDestDir))

+            {

+                Directory.CreateDirectory(dictionaryDestDir);

+            }

+

+            foreach (string file in files)

+            {

+                File.Copy(Path.Combine(dictionarySourceDir, file),

+                          Path.Combine(dictionaryDestDir, file), true);

+

+            }

+        }

+        

 		[TestFixtureTearDown]

 		public void FixtureTearDown()

 		{

@@ -199,5 +205,15 @@ namespace Enchant.Tests
 			List<string> suggestions = new List<string>(dictionary.Suggest("helo"));

 			Assert.Contains("hello", suggestions);

 		}

+

+        [Test]

+        public void Dispose_Called_SendsDisposedEvent()

+        {

+            bool disposedEventCalled = false;

+            dictionary.Disposed += delegate

+                                   { disposedEventCalled = true; };

+            dictionary.Dispose();

+            Assert.IsTrue(disposedEventCalled);

+        }

 	}

 }
\ No newline at end of file