JDK-8223313 : Use handshakes for CountStackFrames.
  • Type: Enhancement
  • Component: hotspot
  • Sub-Component: runtime
  • Affected Version: 13
  • Priority: P4
  • Status: Resolved
  • Resolution: Fixed
  • Submitted: 2019-05-03
  • Updated: 2019-08-15
  • Resolved: 2019-05-21
The Version table provides details related to the release that this issue/RFE will be addressed.

Unresolved : Release in which this issue/RFE will be addressed.
Resolved: Release in which this issue/RFE has been resolved.
Fixed : Release in which this issue/RFE has been fixed. The release containing this fix may be available for download as an Early Access Release or a General Availability Release.

To download the current JDK release, click here.
JDK 13
13 b22Fixed
Related Reports
Relates :  
Description
It's easy to use a handshake instead, making it one less use of suspend resume.
countStackFrames() is deprecated and docs say you must suspend the thread.
Unfortunately we need to emulate that in the handhake.

Comments
A clarification (from the review thread): The reason for the "is_thread_fully_suspended(true," call is that a call to SuspendThread() will return to the caller once an external suspend is requested. The target thread does not have to have completed the self-suspension, but it does have to not execute any more bytecode or bytecode equivalents. So this sequence: SuspendThread(target); CountStackFrames(target); requires that CountStackFrames() wait for target's stack to be safe to walk. In the cooperative self-suspension world, that means waiting for the suspend request to be complete. Without "is_thread_fully_suspended(true," (in the current system), it is possible for CountStackFrames() to crash even without a rogue resume.
21-05-2019

Semantically countStackFrames() is only intended for use on suspended threads - otherwise it is a pointless question to ask a thread - so it is not based on the implementation. I still get the sense that we shouldn't actually need to use is_thread_fully_suspended here as that seems to be trying to account for two different threads doing the suspend and the countStackFrames such that there might be a race between them. That seems a pointless usecase to try and support. It's unfortunate that we need to make any changes in such ancient code, but I agree you should go with whatever is simplest whilst preserving existing semantics.
07-05-2019

Yes, the handshake guards against the resume. I would prefer to skip the suspend test, just count the frames and change the docs for it. But since this is deprecated no one should be using it, so this was the simplest way to remove the is_thread_fully_suspended call without changing behavior. (I assume that requirement for suspended was because of the implementation.) We could check suspend before doing the handshake also. But again no one should be using this.
06-05-2019

I don't quite see the point in this proposed change. The target thread must be suspended (which itself will be done via a handshake eventually) so we don't need an actual handshake to count the stackframes. All we need is a way to accurately ask "is the thread actually suspended". Or are we trying to guard against a resume in the middle of doing that?
04-05-2019

http://cr.openjdk.java.net/~rehn/8223313/webrev/
03-05-2019