On Sat, 15 Apr 2023 10:15:20 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.)
>
> Kevin Walls has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   comment update feedback

Thanks for the comments and reviews - the updated Panama situation is that this 
test as written will still fail sometimes, but that is because the test is 
doing it wrong.  An actual call to the native GetLastError can still overwrite 
the last error value.  (Making a new call is likely to break the last error 
value just doing method resolution, at least the first time it happens.)

But the answer to the original problem is that we now have 
Linker.Option.CaptureCallState which gives us the chance to capture last error 
when calling a MethodHandle, and read the stored last error code in a 
VarHandle.  

I should remove the test, it is redundant, calling set/get last error directly 
is not the way to do this, and CaptureCallState is tested in 
test/jdk/java/foreign/capturecallstate/TestCaptureCallState.java

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

PR Comment: https://git.openjdk.org/jdk/pull/13481#issuecomment-1533093670

Reply via email to