On 6/1/20 12:10 AM, David Holmes wrote:
Hi Daniil,If this change is really just diagnostic in nature, then it should be a subtask. However, it seems to me it will actually hide the failure. The test won't get a timeout and won't print the log. Am I missing something?
On 30/05/2020 10:07 am, Daniil Titov wrote:
Please review a change [1] that fixes an intermittent test timeout.
The main logic of the test has this basic structure:
try {
// lots of thread state manipulation of target
}
finally {
thread.getLog();
}
and as David noticed in his comment ( the last comment in [2] ) if an exception occurs anywhere
in the try block we can hang waiting for the join() in getLog() because we haven't executed the logic that
tells the thread to terminate.
So the fix puts a timeout on the join() which means the test will no longer timeout but it will still fail when whatever was leading to the timeout now happens. So as a diagnostic fix this seems fine. Hopefully the logger will show what we need to see and determine the real underlying problem.
Also, after reading through the bug comments it looks like the getLog()/join() timeout issue is different from the main issue that caused the CR to be filed in the first place. Comments regarding the initial problem are:
"According to the stack trace the test seems to hang on trying to load the 'java.lang.Math' class concurrently. "
"Need to see some native stacks to understand why the classloading thread is not proceeding even though RUNNABLE."
"I should have looked at the test first - it uses Thread.suspend and Thread.resume and so is inherently deadlock prone."
thanks,
Chris
Thanks,
David
-----
Testing: Running a modified test that explicitly throws a runtime exception inside the try block shows the fix solves the problem.
Mach5 tier1-tier3 tests passed. Mach5 tier4-tier5 tests are in progress.
[1] http://cr.openjdk.java.net/~dtitov/8081652/webrev.01/
[2] https://bugs.openjdk.java.net/browse/JDK-8081652
Thank you,
Daniil
