From 32f2ce6fe0f6924de85076b685fb481862bd0bcb Mon Sep 17 00:00:00 2001
From: Eberhard Beilharz <eb1@sil.org>
Date: Fri, 27 Sep 2019 11:49:40 +0200
Subject: [PATCH] Fix crash disposing RuleBasedCollator

Sometimes we got a crash when disposing a RuleBasedCollator. This
change attempts to fix it.
---
 CHANGELOG.md                                  |  4 ++
 source/icu.net/Collation/RuleBasedCollator.cs | 37 +++++++++----------
 2 files changed, 21 insertions(+), 20 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index cd18d01d..52ee8feb 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -22,6 +22,10 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 - Create nuget symbol package
 - Add BiDi class (#121; jeffska)
 
+### Fixed
+
+- Crash on Linux disposing `RuleBasedCollator`
+
 ## [2.5.4] - 2019-01-09
 
 ### Fixed
diff --git a/source/icu.net/Collation/RuleBasedCollator.cs b/source/icu.net/Collation/RuleBasedCollator.cs
index bef3fa47..7610f7e6 100644
--- a/source/icu.net/Collation/RuleBasedCollator.cs
+++ b/source/icu.net/Collation/RuleBasedCollator.cs
@@ -37,7 +37,7 @@ protected override bool ReleaseHandle()
 			{
 				try
 				{
-					if (handle != IntPtr.Zero)
+					if (!IsInvalid)
 						NativeMethods.ucol_close(handle);
 					handle = IntPtr.Zero;
 					return true;
@@ -55,10 +55,7 @@ protected override bool ReleaseHandle()
 			///<returns>
 			///true if the handle is valid; otherwise, false.
 			///</returns>
-			public override bool IsInvalid
-			{
-				get { return handle == IntPtr.Zero; }
-			}
+			public override bool IsInvalid => handle == IntPtr.Zero || handle == new IntPtr(-1) || IsClosed;
 		}
 
 		private bool _disposingValue; // To detect redundant calls
@@ -447,25 +444,25 @@ public override int Compare(string string1, string string2)
 		/// </summary>
 		protected override void Dispose(bool disposing)
 		{
-			if (!_disposingValue)
+			if (_disposingValue)
+				return;
+
+			if (disposing)
 			{
-				if (disposing)
+				// Dispose managed state (managed objects), if any.
+
+				// although SafeRuleBasedCollatorHandle deals with an unmanaged resource
+				// it itself is a managed object, so we shouldn't try to dispose it
+				// if !disposing because that could lead to a corrupt stack (as observed
+				// in https://jenkins.lsdev.sil.org:45192/view/Icu/view/All/job/GitHub-IcuDotNet-Win-any-master-release/59)
+				if (!_collatorHandle.IsInvalid)
 				{
-					// Dispose managed state (managed objects), if any.
-
-					// although SafeRuleBasedCollatorHandle deals with an unmanaged resource
-					// it itself is a managed object, so we shouldn't try to dispose it
-					// if !disposing because that could lead to a corrupt stack (as observed
-					// in https://jenkins.lsdev.sil.org:45192/view/Icu/view/All/job/GitHub-IcuDotNet-Win-any-master-release/59)
-					if (_collatorHandle != default(SafeRuleBasedCollatorHandle))
-					{
-						_collatorHandle.Dispose();
-					}
+					_collatorHandle.Dispose();
 				}
-				_collatorHandle = default(SafeRuleBasedCollatorHandle);
-
-				_disposingValue = true;
 			}
+			_collatorHandle = default;
+
+			_disposingValue = true;
 		}
 
 		/// <summary>