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