[
https://issues.apache.org/jira/browse/SOLR-17000?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17769729#comment-17769729
]
Chris M. Hostetter commented on SOLR-17000:
-------------------------------------------
{quote}Any reason not using ExecutorUtil.shutdownNowAndAwaitTermination?
...
I think I saw "executorService.shutdownNow();" twice in the patch without using
ExecutorUtil.
{quote}
Well ...
# {{ExecutorUtil.shutdownNowAndAwaitTermination}} doesn't take in a timeout,
so the test would have to wait the full 60 seconds
# Even if it did have a timeout – we wouldn't want to use that method, because
it's actually important for the logic of the test that we
{{awaitWorkerInteruptedAtLeastOnce()}} _between_ the call to {{shutdownNow}}
and the call to {{awaitTermination()}}
...but in terms of the last resort cleanup in the {{finally}} blocks ... sure,
that could be {{ExecutorUtil.shutdownNowAndAwaitTermination}} ... i'll update
the patch when i get a chance.
> ExecutorUtilTest failures due to bad concurrency assumptions in test logic
> --------------------------------------------------------------------------
>
> Key: SOLR-17000
> URL: https://issues.apache.org/jira/browse/SOLR-17000
> Project: Solr
> Issue Type: Test
> Security Level: Public(Default Security Level. Issues are Public)
> Reporter: Chris M. Hostetter
> Assignee: Chris M. Hostetter
> Priority: Major
> Attachments: SOLR-17000.patch,
> apache_solr_Solr-check-9.3_647.log.txt, apache_solr_Solr-check-9.3_665.log.txt
>
>
> The basic logic of {{ExecutorUtilTest.testExecutorUtilAwaitsTerminationEnds}}
> in psuedo code is...
> {code:java}
> Future f = executorService.submit(newTaskSleepAndIgnoreInterupts(300ms))
> // L45
> executorService.shutdownNow();
> // L46
> assertThrows(RuntimeException,
> // L47
> ExecutorUtil.awaitTermination(executorService, 100ms)
> // Thread should not have finished in await termination.
>
> assertFalse(f.isDone());
> // L53
> {code}
> There are at least two concurrency assumptions here that are not guaranteed
> to be true, and occasionally cause jenkins failures...
> * There is no guarantee that the task submitted on line 45 will start before
> the {{shutdownNow()}} call on line 46 – which means {{awaitTermination()}}
> can succeed w/o throwing an exception:
> {noformat}
> // apache_solr_Solr-check-9.3_665.log.txt
> org.apache.solr.common.util.ExecutorUtilTest >
> testExecutorUtilAwaitsTerminationEnds FAILED
> java.lang.AssertionError: expected java.lang.RuntimeException to be
> thrown, but nothing was thrown
> at
> __randomizedtesting.SeedInfo.seed([7169EEE284A03087:DFB4E65167A857BF]:0)
> at org.junit.Assert.assertThrows(Assert.java:1028)
> at org.junit.Assert.assertThrows(Assert.java:981)
> at
> org.apache.solr.common.util.ExecutorUtilTest.testExecutorUtilAwaitsTerminationEnds(ExecutorUtilTest.java:47)
> {noformat}
> * From the perspective of the test thread, it's guaranteed that _at least_
> 100ms has elapsed (since line 45) by the time {{awaitTermination()}} throws
> it's exception, but that doesn't preclude the possibility that by the time
> the test thread gets to line 53, a full 300ms may have elapsed, and the
> background thread has already finished the task, so that the
> {{Future.isDone()}} call may return true:
> {noformat}
> // apache_solr_Solr-check-9.3_647.log.txt
> org.apache.solr.common.util.ExecutorUtilTest >
> testExecutorUtilAwaitsTerminationEnds FAILED
> java.lang.AssertionError
> at __randomizedtesting.SeedInfo.seed([F7AFD5785E583017:5972DDCBBD50572F]:0)
> at org.junit.Assert.fail(Assert.java:87)
> at org.junit.Assert.assertTrue(Assert.java:42)
> at org.junit.Assert.assertFalse(Assert.java:65)
> at org.junit.Assert.assertFalse(Assert.java:75)
> at
> org.apache.solr.common.util.ExecutorUtilTest.testExecutorUtilAwaitsTerminationEnds(ExecutorUtilTest.java:53)
> {noformat}
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
