[GitHub] [lucenenet] NightOwl888 opened a new pull request #524: Lucene.Net.Index.IndexWriter::CloseInternal(): Removed call to ThreadJob.Interrupted()

[GitBox <gi...@apache.org>]

NightOwl888 opened a new pull request #524:
URL: https://github.com/apache/lucenenet/pull/524


   `ThreadJob.Interrupted()` was added in commit https://github.com/apache/lucenenet/commit/71d143037a5753edea612c56219288d716b677c2 and was intended to be a direct replacement for Java's [`Thread.interrupted()` method](https://docs.oracle.com/javase/7/docs/api/java/lang/Thread.html#interrupted()).
   
   > public static boolean interrupted()
   >
   > Tests whether the current thread has been interrupted. The interrupted status of the thread is cleared by this method. In other words, if this method were to be called twice in succession, the second call would return false (unless the current thread were interrupted again, after the first call had cleared its interrupted status and before the second call had examined it).
   >
   > A thread interruption ignored because a thread was not alive at the time of the interrupt will be reflected by this method returning false.
   >
   > **Returns:**
   > true if the current thread has been interrupted; false otherwise.
   
   However, unlike Java's `Thread.interrupted()`, `ThreadJob.Interrupted()` internally calls `Thread.Sleep(0)` which causes undesired side effects which cause the [`Lucene.Net.Index.TestIndexWriter::TestThreadInterruptDeadlock()`](https://github.com/apache/lucenenet/blob/8ebd83090165d4688729040ea12ad4ed588bf7bf/src/Lucene.Net.Tests/Index/TestIndexWriter.cs#L1434) and [`Lucene.Net.Index.TestIndexWriter::TestTwoThreadsInterruptDeadlock()`](https://github.com/apache/lucenenet/blob/8ebd83090165d4688729040ea12ad4ed588bf7bf/src/Lucene.Net.Tests/Index/TestIndexWriter.cs#L1476) to intermittently fail.
   
   Since .NET has no way to clear the interrupted status without an undesired side effect, a different approach is required. `CloseInternal()` doesn't use the interrupted status for anything other than calling `Thread.CurrentThread.Interrupt()` to conditionally set the state again after the method runs. However, it appears that in .NET clearing the status is not required since we can rely on the `ThreadInterruptedException` to tell us that the thread was interrupted in order to restore the state at the end of the method.
   
   However, the background threads in the tests do require the interrupted status to be either cleared beforehand or for there to be catch blocks for `ThreadInterruptedException` so it can be ignored.
   
   ### .NET Framework
   
   With these changes, `Lucene.Net.Index.TestIndexWriter::TestThreadInterruptDeadlock()` will run continuously for 500 iterations and still pass.
   
   ### .NET Core
   
   However, on .NET Core both of the mentioned tests still fail around 60% of the time.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@lucenenet.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [lucenenet] NightOwl888 merged pull request #524: Lucene.Net.Index.IndexWriter::CloseInternal(): Removed call to ThreadJob.Interrupted()

[GitBox <gi...@apache.org>]

NightOwl888 merged pull request #524:
URL: https://github.com/apache/lucenenet/pull/524


   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@lucenenet.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org