8254668: JVMTI process frames on thread without started processing #627

stefank · 2020-10-13T09:25:55Z

I hit the following assert in some tests runs that I've been doing:

# Internal Error (/home/stefank/git/alt/open/src/hotspot/share/runtime/stackWatermark.inline.hpp:67), pid=828170, tid=828734
# assert(processing_started()) failed: Processing should already have started

The stack traces for this has been:

Native frames: (J=compiled Java code, A=aot compiled Java code, j=interpreted, Vv=VM code, C=native code)
V [libjvm.so+0x1626d75] StackWatermarkSet::on_iteration(JavaThread*, frame const&)+0xd5
V [libjvm.so+0xad791a] frame::sender(RegisterMap*) const+0x7a
V [libjvm.so+0xacd3f8] frame::real_sender(RegisterMap*) const+0x18
V [libjvm.so+0x1804c4a] vframe::sender() const+0xea
V [libjvm.so+0x175f47b] JavaThread::last_java_vframe(RegisterMap*)+0x5b
V [libjvm.so+0x10e10fc] JvmtiEnvBase::vframeFor(JavaThread*, int)+0x4c
V [libjvm.so+0x10e6972] JvmtiEnvBase::check_top_frame(Thread*, JavaThread*, jvalue, TosState, Handle*)+0xe2
V [libjvm.so+0x10e759c] JvmtiEnvBase::force_early_return(JavaThread*, jvalue, TosState)+0x11c
V [libjvm.so+0x105b8f5] jvmti_ForceEarlyReturnObject+0x215

V  [libjvm.so+0x1626d75]  StackWatermarkSet::on_iteration(JavaThread*, frame const&)+0xd5
V  [libjvm.so+0xad791a]  frame::sender(RegisterMap*) const+0x7a
V  [libjvm.so+0xacd3f8]  frame::real_sender(RegisterMap*) const+0x18
V  [libjvm.so+0x1804c4a]  vframe::sender() const+0xea
V  [libjvm.so+0x1804d00]  vframe::java_sender() const+0x10
V  [libjvm.so+0x10e1115]  JvmtiEnvBase::vframeFor(JavaThread*, int)+0x65
V  [libjvm.so+0x10d475f]  JvmtiEnv::NotifyFramePop(JavaThread*, int)+0x9f
V  [libjvm.so+0x106b6aa]  jvmti_NotifyFramePop+0x23a

The code inspects the top frame of a suspended java thread. However, there's nothing in the code that starts the watermark processing of the thread, so the code asserts when sender calls on_iteration.

We only have to call start_processing/on_iteration when oops are being read. The failing code does not inspect any oops, so I turn of the on_iteration call by settings process_frame to false.

To notify the readers of the code that vframeFor doesn't process the oops, I've renamed the function to vframeForNoProcess to give a visual cue.

I found this bug when running this command line:
makec ../build/fastdebug/ test TEST=test/hotspot/jtreg/vmTestbase/nsk/jvmti JTREG="JAVA_OPTIONS=-XX:+UseZGC -Xmx2g -XX:ZCollectionInterval=1 -XX:ZFragmentationLimit=0.01" JTREG_EXTRA_PROBLEM_LISTS=ProblemList-zgc.txt

Five tests consistently asserts with this command line. All tests pass with the proposed fix.

Recommendations of tests to run are welcome. I intend to get this run through tier1-3, but haven't yet.

Progress

Change must not contain extraneous whitespace
Commit message must refer to an issue
Change must be properly reviewed

Testing

	Linux x64	Windows x64	macOS x64
Build	✔️ (5/5 passed)	✔️ (2/2 passed)	✔️ (2/2 passed)
Test (tier1)	✔️ (9/9 passed)	✔️ (9/9 passed)	✔️ (9/9 passed)

Issue

JDK-8254668: JVMTI process frames on thread without started processing

Reviewers

Erik Österlund (@fisk - Reviewer) ⚠️ Review applies to 587fd35
Richard Reingruber (@reinrich - Committer) ⚠️ Review applies to 587fd35

Download

$ git fetch https://git.openjdk.java.net/jdk pull/627/head:pull/627
$ git checkout pull/627

bridgekeeper · 2020-10-13T09:26:36Z

👋 Welcome back stefank! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

stefank · 2020-10-13T09:27:48Z

Notifying @reinrich @fisk since I think they have been looking into similar problems.

openjdk · 2020-10-13T09:28:54Z

@stefank The following labels will be automatically applied to this pull request:

hotspot
serviceability

When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing lists. If you would like to change these labels, use the /label pull request command.

mlbridge · 2020-10-13T09:33:23Z

Webrevs

01: Full - Incremental (00c1b25)
00: Full (587fd35)

fisk

Looks good. Thanks for fixing this.

openjdk · 2020-10-13T11:28:36Z

@stefank This change now passes all automated pre-integration checks.

ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details.

After integration, the commit message for the final commit will be:

8254668: JVMTI process frames on thread without started processing

Reviewed-by: eosterlund, rrich

You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed.

At the time when this comment was updated there had been 6 new commits pushed to the master branch:

65393a0: 8229867: Re-examine synchronization usages in http and https protocol handlers
6fe209b: 8254671: ZGC: Remove unused roots iterator types
9c93490: 8254177: (tz) Upgrade time-zone data to tzdata2020b
5d6a625: 8254576: ZGC: Clean up timers in roots iterators
508c8a9: 8247591: Document Alpine Linux build steps in OpenJDK build guide
63009f9: 8247589: Implementation of Alpine Linux/x64 Port

Please see this link for an up-to-date comparison between the source branch of this pull request and the master branch.
As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details.

➡️ To integrate this PR with the above commit message to the master branch, type /integrate in a new comment.

reinrich

Hi Stefan,

thanks for fixing.

With this change the assertion in pr #119 does not fail anymore.

The fix looks good to me but I'm not an ZGC expert, neither a Reviewer :)

src/hotspot/share/prims/jvmtiEnvBase.cpp

stefank · 2020-10-13T13:03:09Z

Thanks @fisk and @reinrich for reviewing.

stefank · 2020-10-14T10:28:13Z

/integrate

openjdk · 2020-10-14T10:29:34Z

@stefank Since your change was applied there have been 19 commits pushed to the master branch:

dc262df: 8212218: [TESTBUG] runtime/ErrorHandling/TestHeapDumpOnOutOfMemoryErrorInMetaspace.java timed out
9eeeb8a: 8254696: safepointMechanism_aix needs adaptation for JDK-8253180
b509e31: 8254602: compiler/debug/TestStressCM.java failed with "RuntimeException: got the same optimization stats for different seeds: expected 45"
9fe9b24: 8254575: C2: Clean up unused TRACK_PHI_INPUTS assertion code
31d9b7f: 8254252: Generic arraycopy stub overwrites callee-save rdi register on 64-bit Windows
a098037: 8254365: ciMethod.hpp should not include methodHandles.hpp
d50e0de: 8254722: bsd_zero builds broken after JDK-8253717
ba5dc67: 8254158: Consolidate per-platform stack overflow handling code
715e24a: 8254311: Incorrect statements in createWindowsDevkit2017.sh
ba24f96: 8251861: Remove unused jdk.internal.ref.SoftCleanable and WeakCleanable
... and 9 more: https://git.openjdk.java.net/jdk/compare/9d230ea87dac793a10b256aa773d7fa2262057f0...master

Your commit was automatically rebased without conflicts.

Pushed as commit db9dcdf.

💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.

8254668: JVMTI process frames on thread without started processing

587fd35

openjdk bot added the rfr Pull request is ready for review label Oct 13, 2020

openjdk bot added serviceability serviceability-dev@openjdk.org hotspot hotspot-dev@openjdk.org labels Oct 13, 2020

fisk approved these changes Oct 13, 2020

View reviewed changes

openjdk bot added the ready Pull request is ready to be integrated label Oct 13, 2020

reinrich approved these changes Oct 13, 2020

View reviewed changes

src/hotspot/share/prims/jvmtiEnvBase.cpp Outdated Show resolved Hide resolved

Review 1

00c1b25

reinrich mentioned this pull request Oct 13, 2020

8227745: Enable Escape Analysis for Better Performance in the Presence of JVMTI Agents #119

Closed

3 tasks

openjdk bot closed this Oct 14, 2020

openjdk bot added integrated Pull request has been integrated and removed ready Pull request is ready to be integrated rfr Pull request is ready for review labels Oct 14, 2020

stefank deleted the 8254668_jvmti_inspect_frame_no_process branch October 14, 2020 10:42

8254668: JVMTI process frames on thread without started processing #627

8254668: JVMTI process frames on thread without started processing #627

stefank commented Oct 13, 2020 •

edited by openjdk bot

bridgekeeper bot commented Oct 13, 2020

stefank commented Oct 13, 2020

openjdk bot commented Oct 13, 2020

mlbridge bot commented Oct 13, 2020 •

edited

fisk left a comment

openjdk bot commented Oct 13, 2020 •

edited

reinrich left a comment

stefank commented Oct 13, 2020

stefank commented Oct 14, 2020

openjdk bot commented Oct 14, 2020

8254668: JVMTI process frames on thread without started processing #627

8254668: JVMTI process frames on thread without started processing #627

Conversation

stefank commented Oct 13, 2020 • edited by openjdk bot

Progress

Testing

Issue

Reviewers

Download

bridgekeeper bot commented Oct 13, 2020

stefank commented Oct 13, 2020

openjdk bot commented Oct 13, 2020

mlbridge bot commented Oct 13, 2020 • edited

Webrevs

fisk left a comment

Choose a reason for hiding this comment

openjdk bot commented Oct 13, 2020 • edited

reinrich left a comment

Choose a reason for hiding this comment

stefank commented Oct 13, 2020

stefank commented Oct 14, 2020

openjdk bot commented Oct 14, 2020

stefank commented Oct 13, 2020 •

edited by openjdk bot

mlbridge bot commented Oct 13, 2020 •

edited

openjdk bot commented Oct 13, 2020 •

edited