[SOLR-17000] ExecutorUtilTest failures due to bad concurrency assumptions in test logic

• Type: Test
• Status: Closed
• Priority: Major
• Resolution: Fixed
• Fix Version/s: main (10.0), 9.4
• Assignee: Chris M. Hostetter
• Reporter: Chris M. Hostetter
• Created: 26/Sep/23 18:42
• Updated: 15/Oct/23 20:13
• Resolved: 03/Oct/23 21:10

Description

The basic logic of ExecutorUtilTest.testExecutorUtilAwaitsTerminationEnds in psuedo code is...

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

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:

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

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

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

Issue Links

• is caused by: SOLR-16187 ExecutorUtil#awaitTermination shouldn't wait forever