Is _thread_in_native safe for handshake/safepoint?

[Reingruber, Richard]

Hi Robbin,

thanks for picking up the questions.

> I think the main reason for not making the stack walkable is to avoid
> register flushes. Since we do not have such hardware anymore in mainline
> I see no reason to skip making the stack walkable.

Yeah, avoiding register flushes would be a reason but practically always the
stack is made walkable before changing to _thread_in_native (also in 11u which
still supports SPARC). That's why I asked myself if there are code paths which
don't do that.

> I find two places, JFR and ~ThreadInVMfromNative().
> The JavaThreadInVMAndNative should be replaced.

Ok.

> In case of ThreadInVMfromNative I guess the idea is that when going from
> java -> native we already did the stack walkable and then calling e.g. 
> JNI we avoid it. And you are probably going to do many JNI calls.

> So fixing those two maybe we can simplify to:
>  >   650	  case _thread_in_native:
>  >   654	  case _thread_blocked:
>  >   657	    assert(!thread->has_last_Java_frame() || 
> thread->frame_anchor()->walkable(), "blocked and not walkable");
>  >   658	    return true;

> Which would be very nice.

That would be surely very nice :)

But what needs to be fixed in ThreadInVMfromNative? I suppose the (java part of
the) stack is walkable when the instance is created or there is no frame
on stack. This could be asserted like in line 657 above. Then it is still
walkable when the instance is destructed, isn't it?

> We are looking into some simplifications of transitions, since after 
> "8203469: Faster safepoints" blocked and native are almost identical.

> Essential two super-states thread-safe (blocked/native) and 
> thread-unsafe(all other). Having the same 'rules' for walkable stack 
> certainly helps.

I see.

> Does this answer your question(s)?

To some extent. Unfortunately I haven't understood your answers
completely. Anyways thanks!

Cheers, Richard.

-----Original Message-----
From: Robbin Ehn <robbin.ehn at oracle.com> 
Sent: Dienstag, 27. Oktober 2020 09:16
To: Reingruber, Richard <richard.reingruber at sap.com>; Hotspot dev runtime <hotspot-runtime-dev at openjdk.java.net>
Subject: Re: Is _thread_in_native safe for handshake/safepoint?

Hi Richard,

I think the main reason for not making the stack walkable is to avoid
register flushes. Since we do not have such hardware anymore in mainline
I see no reason to skip making the stack walkable.

I find two places, JFR and ~ThreadInVMfromNative().
The JavaThreadInVMAndNative should be replaced.
In case of ThreadInVMfromNative I guess the idea is that when going from
java -> native we already did the stack walkable and then calling e.g. 
JNI we avoid it. And you are probably going to do many JNI calls.

So fixing those two maybe we can simplify to:
 >   650	  case _thread_in_native:
 >   654	  case _thread_blocked:
 >   657	    assert(!thread->has_last_Java_frame() || 
thread->frame_anchor()->walkable(), "blocked and not walkable");
 >   658	    return true;

Which would be very nice.

We are looking into some simplifications of transitions, since after 
"8203469: Faster safepoints" blocked and native are almost identical.

Essential two super-states thread-safe (blocked/native) and 
thread-unsafe(all other). Having the same 'rules' for walkable stack 
certainly helps.

Does this answer your question(s)?

Thanks, Robbin

On 2020-10-22 16:22, Reingruber, Richard wrote:
> Hi,
> 
> I don't understand the method safepoint_safe_with() in safepoint.cpp
> Maybe somebody has a quick answer to the questions below...
> 
>   648	static bool safepoint_safe_with(JavaThread *thread, JavaThreadState state) {
>   649	  switch(state) {
>   650	  case _thread_in_native:
>   651	    // native threads are safe if they have no java stack or have walkable stack
>   652	    return !thread->has_last_Java_frame() || thread->frame_anchor()->walkable();
>   653	
>   654	  case _thread_blocked:
>   655	    // On wait_barrier or blocked.
>   656	    // Blocked threads should already have walkable stack.
>   657	    assert(!thread->has_last_Java_frame() || thread->frame_anchor()->walkable(), "blocked and not walkable");
>   658	    return true;
>   659	
>   660	  default:
>   661	    return false;
>   662	  }
>   663	}
> 
> I thought _thread_in_native and _thread_blocked are both equally safe for
> handshakes/safepoints but that method makes a distinction. Looking at line 652
> it seems possible that a thread is _thread_in_native without a walkable
> stack. The thread is then of course considered not safe. Native wrappers, native
> interpreter entries, ThreadToNativeFromVM change the state to _thread_in_native
> after making the stack walkable though (I looked also at the old sparc versions).
> 
> So how can a thread be _thread_in_native without a walkable stack? And why does
> that state make sense when it blocks handshakes/safepoints?
> 
> I found JavaThreadInVMAndNative which seems to set the state to
> _thread_in_native without making the stack walkable. It seems though that from
> there ThreadInVMfromNative is reachable which can trigger an assertion in
> JavaThread::check_safepoint_and_suspend_for_native_trans() if the stack isn't
> walkable.
> 
> JfrRecorderService::invoke_safepoint_write() : void
> 	JfrRecorderService::write() : void
> 		JfrRecorderService::finalize_current_chunk() : void
> 			JfrRecorderService::chunk_rotation() : void
> 				JfrRecorderService::rotate(int) : void
> 					JfrEmergencyDump::on_vm_shutdown(bool) : void
> 
> Thanks, Richard.
>