NightOwl888 commented on code in PR #1096:
URL: https://github.com/apache/lucenenet/pull/1096#discussion_r1912498569
##########
src/Lucene.Net.TestFramework/Support/Util/RandomizedContext.cs:
##########
@@ -37,6 +40,12 @@ internal class RandomizedContext
private readonly long randomSeed;
private readonly string randomSeedAsHex;
private readonly long testSeed;
+ private List<IDisposable>? toDisposeAtEnd = null;
Review Comment:
I was partially following the upstream code.
https://github.com/randomizedtesting/randomizedtesting/blob/master/randomized-runner/src/main/java/com/carrotsearch/randomizedtesting/RandomizedContext.java#L150-L160.
The `DisposeResources()` method will always be called exactly once on each
test and once on each suite (class), so I suppose the lock is not strictly
necessary, for now. We took a very different approach, though. Our
`RandomizedContext` instance is stored inside of the `NUnit` test or suite
instance. So, it is not very likely multiple threads will access the same
instance of `RandomizedContext` and if so, they will likely be threads applying
to a single test. It will return to the main thread to dispose.
Disposal order is FIFO, since they also used a `List<T>` in Java.
https://github.com/randomizedtesting/randomizedtesting/blob/master/randomized-runner/src/main/java/com/carrotsearch/randomizedtesting/RandomizedContext.java#L59C11-L60
However, they are removing the list from the collection in
`closeResources()` inside of the lock before removing anything from the list.
So, to be on the safe side, we should probably nullify that field after
grabbing the instance and then dispose outside of the lock.
--
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-unsubscr...@lucenenet.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
