You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucenenet.apache.org by sy...@apache.org on 2015/01/06 04:46:25 UTC
[09/18] lucenenet git commit: Fix NativeFSLock, round 1
Fix NativeFSLock, round 1
We should be making locks properly based on the characteristics of the CLR and Windows
All relevant unit tests are now green
Project: http://git-wip-us.apache.org/repos/asf/lucenenet/repo
Commit: http://git-wip-us.apache.org/repos/asf/lucenenet/commit/8a999e1f
Tree: http://git-wip-us.apache.org/repos/asf/lucenenet/tree/8a999e1f
Diff: http://git-wip-us.apache.org/repos/asf/lucenenet/diff/8a999e1f
Branch: refs/heads/master
Commit: 8a999e1f6629ffa16d51bf7ebef8f303d5c7ca3f
Parents: cb2dfdc
Author: Itamar Syn-Hershko <it...@code972.com>
Authored: Mon Jan 5 17:47:48 2015 +0200
Committer: Itamar Syn-Hershko <it...@code972.com>
Committed: Mon Jan 5 17:47:48 2015 +0200
----------------------------------------------------------------------
src/Lucene.Net.Core/Store/Lock.cs | 7 +-
.../Store/NativeFSLockFactory.cs | 246 ++++---------------
.../Util/LuceneTestCase.cs | 1 -
.../core/Store/TestLockFactory.cs | 27 +-
4 files changed, 62 insertions(+), 219 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/lucenenet/blob/8a999e1f/src/Lucene.Net.Core/Store/Lock.cs
----------------------------------------------------------------------
diff --git a/src/Lucene.Net.Core/Store/Lock.cs b/src/Lucene.Net.Core/Store/Lock.cs
index a0682c8..722d9fc 100644
--- a/src/Lucene.Net.Core/Store/Lock.cs
+++ b/src/Lucene.Net.Core/Store/Lock.cs
@@ -32,9 +32,7 @@ namespace Lucene.Net.Store
/// }.run();
/// </pre>
/// </summary>
- /// <seealso cref= Directory#makeLock(String)
- ///
- /// @lucene.internal </seealso>
+ /// <seealso cref="Directory#makeLock(String)"/>
public abstract class Lock : IDisposable
{
/// <summary>
@@ -61,7 +59,7 @@ namespace Lucene.Net.Store
/// with the "root cause" Exception as to why the lock was
/// not obtained.
/// </summary>
- protected internal Exception FailureReason;
+ public Exception FailureReason { get; protected set; }
/// <summary>
/// Attempts to obtain an exclusive lock within amount of
@@ -121,6 +119,7 @@ namespace Lucene.Net.Store
public virtual void Dispose()
{
+ Release();
}
/// <summary>
http://git-wip-us.apache.org/repos/asf/lucenenet/blob/8a999e1f/src/Lucene.Net.Core/Store/NativeFSLockFactory.cs
----------------------------------------------------------------------
diff --git a/src/Lucene.Net.Core/Store/NativeFSLockFactory.cs b/src/Lucene.Net.Core/Store/NativeFSLockFactory.cs
index e21210a..fd98b94 100644
--- a/src/Lucene.Net.Core/Store/NativeFSLockFactory.cs
+++ b/src/Lucene.Net.Core/Store/NativeFSLockFactory.cs
@@ -1,6 +1,9 @@
+using System;
+using System.Collections.Concurrent;
+using Lucene.Net.Util;
+
namespace Lucene.Net.Store
{
- using System.Collections.Generic;
using System.IO;
/*
@@ -112,13 +115,11 @@ namespace Lucene.Net.Store
internal class NativeFSLock : Lock
{
- private System.IO.FileStream f;
private FileStream Channel;
- private bool @lock;
- private DirectoryInfo Path;
- private DirectoryInfo LockDir;
+ private readonly DirectoryInfo Path;
+ private readonly DirectoryInfo LockDir;
- /*
+ /*
* The javadocs for FileChannel state that you should have
* a single instance of a FileChannel (per JVM) for all
* locking against a given file. To ensure this, we have
@@ -128,7 +129,7 @@ namespace Lucene.Net.Store
* one JVM (each with their own NativeFSLockFactory
* instance) have set the same lock dir and lock prefix.
*/
- private static HashSet<string> LOCK_HELD = new HashSet<string>();
+ private static readonly ConcurrentDictionary<string, WeakReference<FileStream>> LOCKS_HELD = new ConcurrentDictionary<string, WeakReference<FileStream>>();
public NativeFSLock(DirectoryInfo lockDir, string lockFileName)
{
@@ -136,32 +137,19 @@ namespace Lucene.Net.Store
Path = new DirectoryInfo(System.IO.Path.Combine(lockDir.FullName, lockFileName));
}
- private bool LockExists()
- {
- lock (this)
- {
- return @lock != false;
- }
- }
-
public override bool Obtain()
{
lock (this)
{
- if (LockExists())
+ if (Channel != null)
{
// Our instance is already locked:
return false;
}
- // Ensure that lockDir exists and is a directory.
- bool tmpBool;
- if (System.IO.File.Exists(LockDir.FullName))
- tmpBool = true;
- else
- tmpBool = System.IO.Directory.Exists(LockDir.FullName);
+ //LOCKS_HELD.GetOrAdd(Path.FullName)
- if (!tmpBool)
+ if (!System.IO.Directory.Exists(LockDir.FullName))
{
try
{
@@ -172,192 +160,57 @@ namespace Lucene.Net.Store
throw new System.IO.IOException("Cannot create directory: " + LockDir.FullName);
}
}
- else if (!System.IO.Directory.Exists(LockDir.FullName))
+ else if (System.IO.File.Exists(LockDir.FullName))
{
throw new System.IO.IOException("Found regular file where directory expected: " + LockDir.FullName);
}
- string canonicalPath = Path.FullName;
-
- bool markedHeld = false;
-
+ bool success = false;
try
{
- // Make sure nobody else in-process has this lock held
- // already, and, mark it held if not:
-
- lock (LOCK_HELD)
- {
- if (LOCK_HELD.Contains(canonicalPath))
- {
- // Someone else in this JVM already has the lock:
- return false;
- }
- else
- {
- // This "reserves" the fact that we are the one
- // thread trying to obtain this lock, so we own
- // the only instance of a channel against this
- // file:
- LOCK_HELD.Add(canonicalPath);
- markedHeld = true;
- }
- }
-
- try
- {
- f = new System.IO.FileStream(Path.FullName, System.IO.FileMode.Create, System.IO.FileAccess.Write);
- }
- catch (System.IO.IOException e)
- {
- // On Windows, we can get intermittent "Access
- // Denied" here. So, we treat this as failure to
- // acquire the lock, but, store the reason in case
- // there is in fact a real error case.
- FailureReason = e;
- f = null;
- }
- // lucene.net: UnauthorizedAccessException does not derive from IOException like in java
- catch (System.UnauthorizedAccessException e)
- {
- // On Windows, we can get intermittent "Access
- // Denied" here. So, we treat this as failure to
- // acquire the lock, but, store the reason in case
- // there is in fact a real error case.
- FailureReason = e;
- f = null;
- }
-
- if (f != null)
+ Channel = new FileStream(Path.FullName, FileMode.Create, FileAccess.Write, FileShare.None);
+ Channel.Lock(0, Channel.Length);
+ success = true;
+ }
+ catch (IOException e)
+ {
+ FailureReason = e;
+ Channel = null;
+ }
+ // LUCENENET: UnauthorizedAccessException does not derive from IOException like in java
+ catch (System.UnauthorizedAccessException e)
+ {
+ // On Windows, we can get intermittent "Access
+ // Denied" here. So, we treat this as failure to
+ // acquire the lock, but, store the reason in case
+ // there is in fact a real error case.
+ FailureReason = e;
+ Channel = null;
+ }
+ finally
+ {
+ if (!success)
{
try
{
- Channel = f;
- @lock = false;
- try
- {
- Channel.Lock(0, Channel.Length);
- @lock = true;
- }
- catch (System.IO.IOException e)
- {
- // At least on OS X, we will sometimes get an
- // intermittent "Permission Denied" IOException,
- // which seems to simply mean "you failed to get
- // the lock". But other IOExceptions could be
- // "permanent" (eg, locking is not supported via
- // the filesystem). So, we record the failure
- // reason here; the timeout obtain (usually the
- // one calling us) will use this as "root cause"
- // if it fails to get the lock.
- FailureReason = e;
- }
- // lucene.net: UnauthorizedAccessException does not derive from IOException like in java
- catch (System.UnauthorizedAccessException e)
- {
- // At least on OS X, we will sometimes get an
- // intermittent "Permission Denied" IOException,
- // which seems to simply mean "you failed to get
- // the lock". But other IOExceptions could be
- // "permanent" (eg, locking is not supported via
- // the filesystem). So, we record the failure
- // reason here; the timeout obtain (usually the
- // one calling us) will use this as "root cause"
- // if it fails to get the lock.
- FailureReason = e;
- }
- finally
- {
- if (@lock == false)
- {
- try
- {
- Channel.Close();
- }
- finally
- {
- Channel = null;
- }
- }
- }
+ IOUtils.CloseWhileHandlingException(Channel);
}
finally
{
- if (Channel == null)
- {
- try
- {
- f.Close();
- }
- finally
- {
- f = null;
- }
- }
- }
- }
- }
- finally
- {
- if (markedHeld && !LockExists())
- {
- lock (LOCK_HELD)
- {
- if (LOCK_HELD.Contains(canonicalPath))
- {
- LOCK_HELD.Remove(canonicalPath);
- }
+ Channel = null;
}
}
}
- return LockExists();
- }
-
- /*
- Channel = FileChannel.open(Path.toPath(), StandardOpenOption.CREATE, StandardOpenOption.WRITE);
- bool success = false;
- try
- {
- @lock = Channel.TryLock();
- success = @lock != null;
- }
- catch (System.IO.IOException | OverlappingFileLockException e)
- {
- // At least on OS X, we will sometimes get an
- // intermittent "Permission Denied" System.IO.IOException,
- // which seems to simply mean "you failed to get
- // the lock". But other System.IO.IOExceptions could be
- // "permanent" (eg, locking is not supported via
- // the filesystem). So, we record the failure
- // reason here; the timeout obtain (usually the
- // one calling us) will use this as "root cause"
- // if it fails to get the lock.
- FailureReason = e;
- }
- finally
- {
- if (!success)
- {
- try
- {
- IOUtils.CloseWhileHandlingException(Channel);
- }
- finally
- {
- Channel = null;
- }
- }
- }
- return @lock != null;
- }*/
+ return Channel != null;
+ }
}
public override void Release()
{
lock (this)
{
- if (LockExists())
+ if (Channel != null)
{
try
{
@@ -365,7 +218,6 @@ namespace Lucene.Net.Store
}
finally
{
- @lock = false;
try
{
Channel.Close();
@@ -373,18 +225,10 @@ namespace Lucene.Net.Store
finally
{
Channel = null;
- try
- {
- f.Close();
- }
- finally
- {
- f = null;
- lock (LOCK_HELD)
- {
- LOCK_HELD.Remove(Path.FullName);
- }
- }
+// lock (LOCK_HELD)
+// {
+// LOCK_HELD.Remove(Path.FullName);
+// }
}
}
bool tmpBool;
@@ -419,7 +263,7 @@ namespace Lucene.Net.Store
// The test for is isLocked is not directly possible with native file locks:
// First a shortcut, if a lock reference in this instance is available
- if (LockExists())
+ if (Channel != null)
{
return true;
}
http://git-wip-us.apache.org/repos/asf/lucenenet/blob/8a999e1f/src/Lucene.Net.TestFramework/Util/LuceneTestCase.cs
----------------------------------------------------------------------
diff --git a/src/Lucene.Net.TestFramework/Util/LuceneTestCase.cs b/src/Lucene.Net.TestFramework/Util/LuceneTestCase.cs
index 9d829ea..66bf0ca 100644
--- a/src/Lucene.Net.TestFramework/Util/LuceneTestCase.cs
+++ b/src/Lucene.Net.TestFramework/Util/LuceneTestCase.cs
@@ -638,7 +638,6 @@ namespace Lucene.Net.Util
{
get
{
- //return ClassNameRule.TestClass;
return typeof(LuceneTestCase);
}
}
http://git-wip-us.apache.org/repos/asf/lucenenet/blob/8a999e1f/src/Lucene.Net.Tests/core/Store/TestLockFactory.cs
----------------------------------------------------------------------
diff --git a/src/Lucene.Net.Tests/core/Store/TestLockFactory.cs b/src/Lucene.Net.Tests/core/Store/TestLockFactory.cs
index d779d39..b74a4c7 100644
--- a/src/Lucene.Net.Tests/core/Store/TestLockFactory.cs
+++ b/src/Lucene.Net.Tests/core/Store/TestLockFactory.cs
@@ -197,22 +197,21 @@ namespace Lucene.Net.Store
[Test]
public virtual void TestNativeFSLockFactory()
{
- //NativeFSLockFactory f = new NativeFSLockFactory(CreateTempDir(LuceneTestCase.TestClass.SimpleName));
- NativeFSLockFactory f = new NativeFSLockFactory(AppSettings.Get("tempDir", Path.GetTempPath()));
+ var f = new NativeFSLockFactory(CreateTempDir("testNativeFsLockFactory"));
f.LockPrefix = "test";
- Lock l = f.MakeLock("commit");
- Lock l2 = f.MakeLock("commit");
+ var l = f.MakeLock("commit");
+ var l2 = f.MakeLock("commit");
- Assert.IsTrue(l.Obtain(), "failed to obtain lock");
+ Assert.IsTrue(l.Obtain(), "failed to obtain lock, got exception: {0}", l.FailureReason);
Assert.IsTrue(!l2.Obtain(), "succeeded in obtaining lock twice");
l.Dispose();
- Assert.IsTrue(l2.Obtain(), "failed to obtain 2nd lock after first one was freed");
+ Assert.IsTrue(l2.Obtain(), "failed to obtain 2nd lock after first one was freed, got exception: {0}", l2.FailureReason);
l2.Dispose();
// Make sure we can obtain first one again, test isLocked():
- Assert.IsTrue(l.Obtain(), "failed to obtain lock");
+ Assert.IsTrue(l.Obtain(), "failed to obtain lock, got exception: {0}", l.FailureReason);
Assert.IsTrue(l.Locked);
Assert.IsTrue(l2.Locked);
l.Dispose();
@@ -224,14 +223,16 @@ namespace Lucene.Net.Store
[Test]
public virtual void TestNativeFSLockFactoryLockExists()
{
- DirectoryInfo tempDir = new DirectoryInfo(AppSettings.Get("tempDir", Path.GetTempPath()));
- FileInfo lockFile = new FileInfo(/*tempDir, */"test.lock");
- lockFile.Create();
+ DirectoryInfo tempDir = CreateTempDir("testNativeFsLockFactory");
- Lock l = (new NativeFSLockFactory(tempDir)).MakeLock("test.lock");
- Assert.IsTrue(l.Obtain(), "failed to obtain lock");
+ // Touch the lock file
+ var lockFile = new FileInfo(Path.Combine(tempDir.FullName, "test.lock"));
+ using (lockFile.Create()){};
+
+ var l = (new NativeFSLockFactory(tempDir)).MakeLock("test.lock");
+ Assert.IsTrue(l.Obtain(), "failed to obtain lock, got exception: {0}", l.FailureReason);
l.Dispose();
- Assert.IsFalse(l.Locked, "failed to release lock");
+ Assert.IsFalse(l.Locked, "failed to release lock, got exception: {0}", l.FailureReason);
if (lockFile.Exists)
{
lockFile.Delete();