8269635: Stress test SEGV while emitting OldObjectSample #218

mgronlun · 2021-07-06T14:20:39Z

Greetings,

please help review this change set to protect the unloaded thread id set from growing, and hence de-allocated, in the middle of inspection.

Testing: jdk_jfr

Thanks
Markus

Progress

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

Issue

JDK-8269635: Stress test SEGV while emitting OldObjectSample

Reviewers

Jaroslav Bachorik (@jbachorik - Reviewer)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.java.net/jdk17 pull/218/head:pull/218
$ git checkout pull/218

Update a local copy of the PR:
$ git checkout pull/218
$ git pull https://git.openjdk.java.net/jdk17 pull/218/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 218

View PR using the GUI difftool:
$ git pr show -t 218

Using diff file

Download this PR as a diff file:
https://git.openjdk.java.net/jdk17/pull/218.diff

bridgekeeper · 2021-07-06T14:21:46Z

👋 Welcome back mgronlun! 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.

openjdk · 2021-07-06T14:23:57Z

@mgronlun The following label will be automatically applied to this pull request:

hotspot-jfr

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

mlbridge · 2021-07-06T14:28:00Z

Webrevs

jbachorik · 2021-07-08T17:53:44Z

src/hotspot/share/jfr/leakprofiler/checkpoint/objectSampleCheckpoint.cpp

+void ObjectSampleCheckpoint::clear() {
+  assert(SafepointSynchronize::is_at_safepoint(), "invariant");
+  if (unloaded_thread_id_set != NULL) {
+    delete unloaded_thread_id_set;
+    unloaded_thread_id_set = NULL;
+  }
+  assert(unloaded_thread_id_set == NULL, "invariant");
+}
+


This does not seem to be called anywhere. Is it expected?

Thanks Jaroslav, changes got lost when moving this from main to 17.

openjdk · 2021-07-09T09:41:58Z

@mgronlun 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:

8269635: Stress test SEGV while emitting OldObjectSample

Reviewed-by: jbachorik

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 24 new commits pushed to the master branch:

885f7b1: 8269146: Missing unreported constraints on pattern and other case label combination
62ff55d: 8269952: compiler/vectorapi/VectorCastShape*Test.java tests failed on avx2 machines
46c610c: 8269840: Update Platform.isDefaultCDSArchiveSupported() to return true for aarch64 platforms
6401633: 8269722: NPE in HtmlDocletWriter
9acb2a6: 8270109: ProblemList 4 SA tests on macOS-aarch64
f46a917: 6766844: ByteArrayInputStream#read with a byte array of length 0 not consistent with InputStream when at EOF
9e75f92: 8269738: AssertionError when combining pattern matching and function closure
168af2e: 8269828: corrections in some instruction patterns for KNL x86 platform
fa08cc6: 8268766: Desugaring of pattern matching enum switch should be improved
4f70759: 8270006: Switches with 'case null:' should be exhaustive
... and 14 more: https://git.openjdk.java.net/jdk17/compare/2b20778225527a3ccd3ced2a4246c5943f467e06...master

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.

mgronlun · 2021-07-13T09:42:27Z

/integrate

openjdk · 2021-07-13T09:43:58Z

Going to push as commit bd95c0c.
Since your change was applied there have been 36 commits pushed to the master branch:

00ef65f: 8269525: Deadlock during Volano with JFR
e631add: 8259848: Interim javadoc build does not support platform links
040c02b: 8269795: C2: Out of bounds array load floats above its range check in loop peeling resulting in SEGV
0f32982: 8270203: Missing build dependency between jdk.jfr-gendata and buildtools-hotspot
4fc3180: 8266345: (fs) Custom DefaultFileSystemProvider security related loops
999ced0: 8269873: serviceability/sa/Clhsdb tests are using a C2 specific VMStruct field
e1d3e73: 8268965: TCP Connection Reset when connecting simple socket to SSL server
3d82b0e: 8269558: fix of JDK-8252657 missed to update history at the end of JVM TI spec
2546006: 8270216: [macOS] Update named used for Java run loop mode
6889a39: 8268826: Cleanup Override in Context-Specific Deserialization Filters
... and 26 more: https://git.openjdk.java.net/jdk17/compare/2b20778225527a3ccd3ced2a4246c5943f467e06...master

Your commit was automatically rebased without conflicts.

openjdk · 2021-07-13T09:44:17Z

@mgronlun Pushed as commit bd95c0c.

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

Stress test SEGV while emitting OldObjectSample

e432c9b

openjdk bot added the rfr Pull request is ready for review label Jul 6, 2021

openjdk bot added the hotspot-jfr hotspot-jfr-dev@openjdk.java.net label Jul 6, 2021

clear unloaded_thread_id_set on start to reduce search space

6f57537

jbachorik reviewed Jul 8, 2021

View reviewed changes

mgronlun added 2 commits July 8, 2021 20:49

add missing changes

ebf9c89

move

03bcca6

jbachorik approved these changes Jul 9, 2021

View reviewed changes

openjdk bot added the ready Pull request is ready to be integrated label Jul 9, 2021

openjdk bot closed this Jul 13, 2021

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 Jul 13, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

8269635: Stress test SEGV while emitting OldObjectSample #218

8269635: Stress test SEGV while emitting OldObjectSample #218

Uh oh!

mgronlun commented Jul 6, 2021 •

edited by openjdk bot

Loading

Uh oh!

bridgekeeper bot commented Jul 6, 2021

Uh oh!

openjdk bot commented Jul 6, 2021

Uh oh!

mlbridge bot commented Jul 6, 2021 •

edited

Loading

Uh oh!

jbachorik Jul 8, 2021

Uh oh!

mgronlun Jul 8, 2021 •

edited

Loading

Uh oh!

openjdk bot commented Jul 9, 2021

Uh oh!

mgronlun commented Jul 13, 2021

Uh oh!

openjdk bot commented Jul 13, 2021

Uh oh!

openjdk bot commented Jul 13, 2021

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

2 participants

8269635: Stress test SEGV while emitting OldObjectSample #218

8269635: Stress test SEGV while emitting OldObjectSample #218

Uh oh!

Conversation

mgronlun commented Jul 6, 2021 • edited by openjdk bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Progress

Issue

Reviewers

Reviewing

Uh oh!

bridgekeeper bot commented Jul 6, 2021

Uh oh!

openjdk bot commented Jul 6, 2021

Uh oh!

mlbridge bot commented Jul 6, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Webrevs

Uh oh!

jbachorik Jul 8, 2021

Choose a reason for hiding this comment

Uh oh!

mgronlun Jul 8, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

openjdk bot commented Jul 9, 2021

Uh oh!

mgronlun commented Jul 13, 2021

Uh oh!

openjdk bot commented Jul 13, 2021

Uh oh!

openjdk bot commented Jul 13, 2021

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

2 participants

mgronlun commented Jul 6, 2021 •

edited by openjdk bot

Loading

mlbridge bot commented Jul 6, 2021 •

edited

Loading

mgronlun Jul 8, 2021 •

edited

Loading