On Fri, 10 Dec 2021 07:00:10 GMT, Chris Plummer <cjplum...@openjdk.org> wrote:

> The test searches for "JShellToolProvider" in the main thread's stack trace, 
> which is pulled from an SA heap dump. Typically the main thread is blocked in 
> Object.wait(), so SA can determine its stack trace. However, the wait has a 
> 100ms timeout, so the thread does periodically wake up and does a few things 
> before waiting again. If SA tries to get the stack trace while the thread is 
> active, it may not be able to, and this causes the test to fail. I determined 
> this only happens about 1 in 5000-10000 runs. So as a fix I'm allowing the 
> test to do one retry. That should make it extremely unlikely that we ever see 
> this failure again. I also bumped up the amount of time the test waits before 
> doing the heap dump from 2 seconds to 4 just to make absolutely sure the main 
> thread is fully started before doing the heap dump.

Hi Chris,
It looks good in general. I feel the parameter 'retry' is a little bit 
confusing. It is possible to rename it to some to 'retriedOnce'. Another 
approach would be to name it 'allowRetry' and revert the meaning of boolean 
value. 
I leave it up to you to keep the original approach or make any change.
Thanks,
Serguei

-------------

Marked as reviewed by sspitsyn (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/6795

Reply via email to