Skip to content

Commit

Permalink
Fix crash disposing RuleBasedCollator
Browse files Browse the repository at this point in the history
Sometimes we got a crash when disposing a RuleBasedCollator. This
change attempts to fix it.
  • Loading branch information
ermshiperete committed Sep 27, 2019
1 parent 07eb297 commit 32f2ce6
Show file tree
Hide file tree
Showing 2 changed files with 21 additions and 20 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
37 changes: 17 additions & 20 deletions source/icu.net/Collation/RuleBasedCollator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ protected override bool ReleaseHandle()
{
try
{
if (handle != IntPtr.Zero)
if (!IsInvalid)
NativeMethods.ucol_close(handle);
handle = IntPtr.Zero;
return true;
Expand All @@ -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
Expand Down Expand Up @@ -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>
Expand Down

0 comments on commit 32f2ce6

Please sign in to comment.