You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@lucenenet.apache.org by GitBox <gi...@apache.org> on 2021/08/18 14:55:19 UTC

[GitHub] [lucenenet] rclabo opened a new pull request #513: Two additional ControlledRealTimeReopenThread tests

rclabo opened a new pull request #513:
URL: https://github.com/apache/lucenenet/pull/513


   Added two additional demonstration tests for ControlledRealTimeReopenThread that make it easier to see how to use the class.  One of the tests, TestMultithreadedWaitForGeneration, demonstrates that the current implemention of the ControlledRealTimeReopenThread class is flawed when used in multi-thread situations. This test will fail currently but I will then add another commit that improves the ControlledRealTimeReopenThread class so that it's a more faithful port of the java version and then the test will pass.


-- 
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 commented on a change in pull request #513: Two additional ControlledRealTimeReopenThread tests

Posted by GitBox <gi...@apache.org>.
NightOwl888 commented on a change in pull request #513:
URL: https://github.com/apache/lucenenet/pull/513#discussion_r691515424



##########
File path: src/Lucene.Net/Search/ControlledRealTimeReopenThread.cs
##########
@@ -105,45 +108,38 @@ private void RefreshDone()
         {
             lock (this)
             {
-                // if we're finishing, , make it out so that all waiting search threads will return
-                searchingGen = finish ? long.MaxValue : refreshStartGen;
-                available.Set();
+                searchingGen = refreshStartGen;
+                intrinsic.Set();                        // LUCENENET NOTE:  Will notify all and remain signaled, so it must be reset in WaitForGeneration
             }
-            reopenCond.Reset();
         }
 
         /// <summary>
-        /// Releases all resources used by the <see cref="ControlledRealTimeReopenThread{T}"/>.
+        /// Kills the thread and releases all resources used by the
+        /// <see cref="ControlledRealTimeReopenThread{T}"/>. Also joins to the
+        /// thread so that when this method returns the thread is no longer alive.
         /// </summary>
         public void Dispose()
         {
-            Dispose(true);

Review comment:
       This class is not sealed. We should keep the dispose pattern in place so inheritors have a way to dispose any unmanaged resources that they own and so the finalizer doesn't automatically run.
   
   It would also be wise to keep track of whether `Dispose(bool)` has been called so it is safe to call it more than once. See https://stackoverflow.com/a/5306896. Also, please include tests for multiple-dispose.




-- 
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] rclabo merged pull request #513: Two additional ControlledRealTimeReopenThread tests

Posted by GitBox <gi...@apache.org>.
rclabo merged pull request #513:
URL: https://github.com/apache/lucenenet/pull/513


   


-- 
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] rclabo commented on a change in pull request #513: Two additional ControlledRealTimeReopenThread tests

Posted by GitBox <gi...@apache.org>.
rclabo commented on a change in pull request #513:
URL: https://github.com/apache/lucenenet/pull/513#discussion_r691543315



##########
File path: src/Lucene.Net/Search/ControlledRealTimeReopenThread.cs
##########
@@ -105,45 +108,38 @@ private void RefreshDone()
         {
             lock (this)
             {
-                // if we're finishing, , make it out so that all waiting search threads will return
-                searchingGen = finish ? long.MaxValue : refreshStartGen;
-                available.Set();
+                searchingGen = refreshStartGen;
+                intrinsic.Set();                        // LUCENENET NOTE:  Will notify all and remain signaled, so it must be reset in WaitForGeneration
             }
-            reopenCond.Reset();
         }
 
         /// <summary>
-        /// Releases all resources used by the <see cref="ControlledRealTimeReopenThread{T}"/>.
+        /// Kills the thread and releases all resources used by the
+        /// <see cref="ControlledRealTimeReopenThread{T}"/>. Also joins to the
+        /// thread so that when this method returns the thread is no longer alive.
         /// </summary>
         public void Dispose()
         {
-            Dispose(true);

Review comment:
       Ok, I'll add back a Dispose(bool) method.




-- 
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