On Fri, 14 Apr 2023 19:23:05 GMT, Kevin Walls <kev...@openjdk.org> wrote:
> This test is failing often since 8304725 added a call to > Thread::current_in_asgct(). This can end up being called e.g. when resolving > calls, and then the OS last error value is lost. > > The test is reliable with a single warm-up call to getLastError.invoke() > before the loop. > > The test was introduced when in JDK-8292302 a change was undone that had made > JavaThread::threadObj call Thread::current_or_null_safe, as the use of TLS > upset this case of accessing last error directly. > > This new Thread::current_in_asgct() case shows that the VM will find new ways > to interfere with the last error value, or at least new VM code keeps wanting > to call Thread::current. This testcase is kind of niche usage, so it not an > argument that VM code should not be calling Thread::current. If this test > is to stay active, it needs to have this warm-up getLastError call. (If > there are more issues, it might mean removing the test.) Looks good - thanks for getting to the bottom of this failure! One suggestion below. test/jdk/com/sun/jdi/JdbLastErrorTest.java line 63: > 61: FunctionDescriptor.ofVoid(ValueLayout.JAVA_INT)); > 62: > 63: // One "warmup" call avoids VM activity (e.g. Thread::current) > disturbing last error value. Suggestion: // Perform a "warmup" call to ensure method resolution can't disturb the last error value // that we are trying to set. ------------- Marked as reviewed by dholmes (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/13481#pullrequestreview-1386348947 PR Review Comment: https://git.openjdk.org/jdk/pull/13481#discussion_r1167410330