On Thu, 21 Sep 2023 05:15:26 GMT, Patricio Chilano Mateo 
<pchilanom...@openjdk.org> wrote:

>> `JVM_MoreStackWalk` has a bug that always assumes that the Java frame
>> stream is currently at the frame decoded in the last patch and so always
>> advances to the next frame before filling in the new batch of stack frame.
>> However `JVM_MoreStackWalk` may return 0.  The library will set 
>> the continuation to its parent. It then call `JVM_MoreStackWalk` to continue
>> the stack walking but the last decoded frame has already been advanced.
>> The Java frame stream is already at the top frame of the parent 
>> continuation. .
>> The current implementation skips "Continuation::yield0" mistakenly.  This
>> only happens with `-XX:+ShowHiddenFrames` or 
>> `StackWalker.Option.SHOW_HIDDEN_FRAMES`.
>> 
>> The fix is to pass the number of frames decoded in the last batch to 
>> `JVM_MoreStackWalk`
>> so that the VM will determine if the current frame should be skipped or not.
>> 
>> `test/jdk/jdk/internal/vm/Continuation/Scoped.java` test now correctly checks
>> the expected result where "yield0" exists between "enter" and "run" frames.
>
> src/java.base/share/classes/java/lang/StackStreamFactory.java line 443:
> 
>> 441: 
>> 442:             // If the last batch didn't fetch any frames, keep the 
>> current batch size.
>> 443:             int lastBatchFrameCount = frameBuffer.numFrames();
> 
> I run some tests to understand the issue and I got the same failure if I now 
> set MIN_BATCH_SIZE to 7. This just forces the same situation where 
> Continuation::enter is the last frame in the buffer, otherwise since the 
> patch also changes the batch sizes we don't fall into that issue anymore. The 
> problem is with this numFrames() method which still returns a number > 0 
> after the fetch attempt that returns with no frames. Maybe there is a reset 
> missing for origin and fence when fetching the next batch?

Thanks for catching this.   The problem is that `fetchStackFrames(int 
batchSize)` is supposed to call `setBatch` to set origin and fence for the new 
batch.  But if no frame is fetched, it skips not calling that.   I have a fix 
for it.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/15804#discussion_r1333433443

Reply via email to