RFR: JDK-8257828: SafeFetch may crash if invoked in non-JavaThreads [v2]

[David Holmes]

On 14/12/2020 6:34 pm, Thomas Stuefe wrote:
> On Mon, 14 Dec 2020 04:46:25 GMT, David Holmes <dholmes at openjdk.org> wrote:
> 
>>> Thomas Stuefe has updated the pull request incrementally with one additional commit since the last revision:
>>>
>>>    Make SafeFetch work on Windows + AIX fix
>>
>> One query, but changes can go in as-is.
>> Thanks,
>> David
> 
>> _Mailing list message from [David Holmes](mailto:david.holmes at oracle.com) on [hotspot-runtime-dev](mailto:hotspot-runtime-dev at openjdk.java.net):_
>>
>> On 14/12/2020 3:24 pm, Thomas Stuefe wrote:
>>
>>> On Mon, 14 Dec 2020 04:44:43 GMT, David Holmes <dholmes at openjdk.org> wrote:
>>>>> Thomas Stuefe has updated the pull request incrementally with one additional commit since the last revision:
>>>>> Make SafeFetch work on Windows + AIX fix
>>>>
>>>>
>>>> src/hotspot/os/windows/os_windows.cpp line 2258:
>>>>> 2256:   // Save pc in thread
>>>>> 2257:   if (thread != nullptr && thread->is_Java_thread()) {
>>>>> 2258:     thread->as_Java_thread()->set_saved_exception_pc((address)(DWORD_PTR)exceptionInfo->ContextRecord->PC_NAME);
>>>>
>>>>
>>>> Is this a full fix? If the thread is not a JavaThread we no longer hit a problem in as_Java_thread() but is doing nothing for a non-JavaThread actually the right thing to do?
>>>
>>>
>>> The saved_exception_pc mechanism is only implemented for JavaThreads. But all threads go through the code below, where we change the pc in the context structure.
>>
>> My query is whether it makes sense for only JavaThreads to have this
>> saved (C++?) exception pc? What role does it play and why would it not
>> be applicable to all threads? (Sorry bit of a digression but trying to
>> understand the bigger picture.)
>>
>> Thanks,
>> David
> 
> IIUC, JavaThread::_saved_exception_pc contains the pc from which a safepoint was triggered, and to which we may want to return once the safepoint was handled. Safepoint handling only works for java threads, so limiting this to JavaThread makes sense.

I see now that it relates to the safepoint polling page exception.

Thanks,
David
-----

> -------------
> 
> PR: https://git.openjdk.java.net/jdk/pull/1695
>