Is _thread_in_native safe for handshake/safepoint?

[Robbin Ehn]

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.
>