Dangling CountDownLatch.await() in tests

[leventov]

There are several places in tests where the boolean result of CountDownLatch.await(timeout) is not tested. It should properly be wrapped into assertTrue() in (almost) all cases. Currently, these tests don't really verify the correctness of the production code.

See https://github.com/code-review-checklists/java-concurrency#check-await

To systematically eradicate these bugs, either the configuration for "Result of method call is ignored" inspection should be edited and the inspection is enabled on error level (which I think is preferable because we want to strictly check this inspection anyway), or a separate Structural Search pattern could be easily created.

[leventov]

Other methods:

• ExecutorService.awaitTermination, e. g. in IntermediaryDataManager should probably at least log something if it fails to terminate in the specified time, FYI @jihoonson. The Structural search pattern should be marked as "within type hierarchy".
• AsynchronousChannelGroup.awaitTermination
• ForkJoinPool.awaitQuiescence
• Semaphore.tryAcquire
• Condition.await, "within type hierarchy"
• Lock.tryLock, "within type hierarchy"
• Process.waitFor
• Monitor.enter (Guava)
• RateLimiter.tryAcquire (Guava)

[jihoonson]

@leventov thank you for finding it. Is there a way to forbid it with a code style or an inspection rule?

[leventov]

$x$.awaitTermination($y$, $z$); structural search

[akashvyom]

Hi,
I would like to work on this ticket, I will spend some time getting myself familiar with the codebase and try to track down where to fix this issue.