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

[Thomas Stuefe]

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.

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

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