Disable late attach to self by default #18

pdbain-ibm · 2017-09-14T18:37:18Z

Prevent a JVM from late attaching to itself unless a system property is set
either to "true" or an empty string, per the reference implementation. Enforce
this in Java 9 only. Update existing tests to enable this feature by default so
they will continue to run. Add new Java 9 tests to exercise this feature.

Fixes #126

Signed-off-by: Peter Bain [email protected]

DanHeidinga · 2017-09-15T04:00:44Z

jcl/src/java.base/share/classes/com/ibm/tools/attach/target/AttachHandler.java

@@ -102,6 +102,12 @@
 	private static String nameProperty;
 	private static String pidProperty;
 	private static int numberOfTargets;
+	/*[IF Sidecar19-SE]*/
+	/*[PR 132828 don't allow VM to attach to itself unless explicitly enabled ]*/


This is likely an internal ticket number - can you remove it and add any necessary context to the code or the PR?

DanHeidinga · 2017-09-15T04:14:53Z

test/Java9andUp/playlist.xml

+
+	<test>
+		<featureIds>
+			<featureId>113813</featureId>


What does this featureId mean?

This is the Jazz work item for the refactoring of the Attach API.

Please use an OpenJ9 issue number rather than a internal Jazz #.

DanHeidinga · 2017-09-15T04:21:22Z

I haven't looked at the test code yet - @smlambert can someone on your team take the lead on that?

genie-openj9 · 2017-09-19T01:23:29Z

Can one of the admins verify this patch?

genie-openj9 · 2017-09-19T01:27:30Z

Can one of the admins verify this patch?

genie-openj9 · 2017-09-19T01:32:25Z

Can one of the admins verify this patch?

Escape analysis may have to create initialization code at the method start. If the first block is part of a loop, it will create a new block to hold these trees. However, the block test did not consider whether the first block was in an improper region, resulting in init code being placed inside a loop. Signed-off-by: Nicholas Coughlin <[email protected]>

Adds missing arguments for Java 9 compile to allow visibility to com.ibm.oti.util package. Adds fork=true argument to ensure compiler uses correct Java version and not Ant Java version These changes ensure the tests are compiled with the Java version we specified in the JAVA_BIN environment variable. Signed-off-by: Mike Zhang <[email protected]>

DanHeidinga · 2017-09-22T17:40:32Z

jcl/src/jdk.attach/share/classes/com/ibm/tools/attach/attacher/OpenJ9VirtualMachine.java

 					exc.initCause(e);
 					throw exc;
 				}

 				if (descriptor.id().equals(AttachHandler.getVmId())) {
+					/*[IF Sidecar19-SE]*/
+					String allowattachselfValue = AttachHandler.allowAttachSelf;


I'm not a fan of this mixed all lower case with camelCase. Can this be named allowAttachSelfValue for better readability?

Done, though I inserted an underscore before "value" to indicate that it's the value of allowAttachSelf.

DanHeidinga · 2017-09-22T17:41:19Z

test/Java9andUp/playlist.xml

+
+	<test>
+		<featureIds>
+			<featureId>113813</featureId>


Please use an OpenJ9 issue number rather than a internal Jazz #.

DanHeidinga · 2017-09-22T17:42:42Z

test/Java9andUp/src/org/openj9/test/java9AttachAPI/SelfAttacher.java

+			Properties props = vm.getSystemProperties();
+			props.list(System.out);
+		} catch (AttachNotSupportedException e) {
+			e.printStackTrace();


Should the test be printing this unconditionally or should it be using an existing logger framework?

SelfAttacher is run as a child process whose stdout and stderr are captured by the TargetManager which can log it, though I don't bother doing so in this test.

DanHeidinga · 2017-09-22T17:45:21Z

test/Java9andUp/src/org/openj9/test/java9AttachAPI/SelfAttacher.java

+			String myId = VmIdGetter.getVmId();
+			System.err.println("myId="+myId);
+			boolean found = false;
+			for (int i = 0; i <10 && !found; ++i) {


Is there no better way to get a VIrtualMachine id? Something without looping?

Playlist: updated.

SelfAttacher:
This isn't to get the vmID. It's cycling waiting for the attach API to initialize. There is OpenJ9 internal API to do that, but it's not portable.

DanHeidinga · 2017-09-22T17:46:02Z

test/Java9andUp/src/org/openj9/test/java9AttachAPI/TargetManager.java

+
+import org.testng.log4testng.Logger;
+
+/* This is was copied from org.openj9.test.attachAPI.TargetManager in Java8AndUp.


Why was it copied? Is there a way to avoid duplicating the code?

The original TargetManager is in Java8AndUp, which shares no OpenJ9 test code with Java9AndUp. Maybe @smlambert or @llxia can suggest a better mechanism.

DanHeidinga · 2017-09-22T19:51:20Z

test/Java9andUp/src/org/openj9/test/java9AttachAPI/TargetManager.java

+		return target;
+	}
+
+	static void listIpcDir() {


Can you add javadoc for each of these methods? It looks like this method tries to recursively list all the files and directories under the java.io.tmpdir to the logger.debug() output. Should it only be called if the logger.debug() level is enabled?

Can you add javadoc for each of these methods?
Done. Let me know if you want more explanation.

It looks like this method tries to recursively list all the files and directories under the java.io.tmpdir to the logger.debug() output. Should it only be called if the logger.debug() level is enabled?
Actually, it lists the contents of /tmp/.com_ibm_tools_attach, which will be a few files for each Java process. This method is mostly for post-failure analysis.

DanHeidinga · 2017-09-22T19:56:28Z

test/Java9andUp/src/org/openj9/test/java9AttachAPI/TargetManager.java

+					errOutput += currentLine + "\n";
+				}
+			}
+			while (targetOutReader.ready()) {


I don't think this is the right code. I would expect a loop while readLine() is not returning null.

As mentioned above, I don't want readline() to block.

I don't think this actually stops readline from blocking - the stream is ready as soon as there are characters available.

The readline() will block until there is a newline, but the target process emits complete lines, so any blocking will be brief. The purpose of the code was to handle the case where stdout would close and stderr was still open but not receiving anything.

DanHeidinga · 2017-09-22T19:56:50Z

test/Java9andUp/src/org/openj9/test/java9AttachAPI/TargetManager.java

+		} finally {
+			try {
+				/* get anything that may have been in the buffers */
+				while (targetErrReader.ready()) {


same as above and in other places using ready().

DanHeidinga · 2017-09-22T19:57:13Z

test/Java9andUp/src/org/openj9/test/java9AttachAPI/TestAttachAPI.java

+ * This test must be invoked via testng.  Running the main method will return only the status of the attach API.
+ *
+ */
+@SuppressWarnings({"nls","boxing"})


Please don't use SuppressWarnings.

DanHeidinga · 2017-09-22T19:59:33Z

test/Java9andUp/src_latest/org/openj9/test/java9AttachAPI/VmIdGetter.java

@@ -0,0 +1,30 @@
+/* Copyright (c) 2015, 2017 IBM Corp. and others


Can you explain the difference between src_latest and src_current? I'm not aware of why there are two...

This is a kludge to handle the API differences between the two JCL versions in the builds.

also make sure all streams are closed Signed-off-by: Keith W. Campbell <[email protected]>

Attempt to work around timeouts cloning this repo in the CI builds by cloning less material. This is fine since the build doesn't commit back to that repo (or other repos for that matter) Issue eclipse-openj9#116 Signed-off-by: Dan Heidinga <[email protected]>

Prevent a JVM from late attaching to itself unless a system property is set either to "true" or an empty string, per the reference implementation. Enforce this in Java 9 only. Update existing tests to enable this feature by default so they will continue to run. Add new Java 9 tests to exercise this feature. Signed-off-by: Peter Bain <[email protected]>

Signed-off-by: Peter Bain <[email protected]>

…kout Use shallow clone of the openj9-openjdk-jdk9 repo in travis build

While resolving bootstrap method handles, TypeNotPresentException is currently thrown if a class is not found since we rely upon the MethodType API. But, NoClassDefFoundError should be thrown if a class is not found while resolving bootstrap method handles. This change translates TypeNotPresentException to NoClassDefFoundError while resolving bootstrap method handles. Signed-off-by: Babneet Singh <[email protected]>

Changes for OpenJ9 issue eclipse-openj9#126 Signed-off-by: Peter Bain <[email protected]>

pdbain-ibm · 2017-09-25T16:44:44Z

Pushed the changes, not fully tested yet.

Translate TypeNotPresentException to NoClassDefFoundError

Signed-off-by: Joe Dekoning <[email protected]>

During refactoring of the attach API, certain classes were being replaced due to legacy build scripts and needed to be replaced. This is no longer the case. Signed-off-by: Peter Bain <[email protected]>

Correct escape analysis loop check

update preprocessor builder and nature remove unnecessary builders and natures Signed-off-by: Keith W. Campbell <[email protected]>

…ription Update jcl project description

Remove tooling to restore original attach API code

Replace J9VM and JIT repo references with OpenJ9

Make IdleTuningGcOnIdle optimization is covered/included part of the option(Xtune:virtualized) which grouped the optimizations relevant for virtualized environment. Issue: eclipse-openj9#124 Signed-off-by: Parameswaran Selvam <[email protected]>

Signed-off-by: Ben Walsh <[email protected]>

Signed-off-by: Peter Bain <[email protected]>

Changes for OpenJ9 issue eclipse-openj9#126 Signed-off-by: Peter Bain <[email protected]>

…h_self

pdbain-ibm · 2017-09-27T12:48:55Z

Will open a new pull request for a single commit.

pdbain-ibm · 2017-09-27T15:13:19Z

In re Dan's comment above, no: git wouldn't let me rebase & squash, so I abandoned this pull request and created another.

Adding the detailed build instructions for creating an OpenjDK V9 with OpenJ9 binary for Windows. Created as a completely separate section in the existing build instructions. No attempt to merge instructions for platforms yet. [ci skip] Work supports issue at ibmruntimes/openj9-openjdk-jdk9#18 Issue eclipse-openj9#18 Signed-off-by: Sue Chaplain <[email protected]>

Compile error fix

Adding the detailed build instructions for creating an OpenjDK V9 with OpenJ9 binary for Windows. Created as a completely separate section in the existing build instructions. No attempt to merge instructions for platforms yet. [ci skip] Work supports issue at ibmruntimes/openj9-openjdk-jdk9#18 Issue eclipse-openj9#18 Signed-off-by: Sue Chaplain <[email protected]>

…pe-queries Switch to value type query methods from ClassEnv

DanHeidinga reviewed Sep 15, 2017

View reviewed changes

pshipton added the comp:vm label Sep 20, 2017

Nicholas Coughlin and others added 2 commits September 21, 2017 11:11

tajila mentioned this pull request Sep 22, 2017

Crash running gradle reported on StackOverflow #110

Closed

DanHeidinga requested changes Sep 22, 2017

View reviewed changes

keithc-ca and others added 4 commits September 22, 2017 16:35

Simplify creation of a socket connection to the attacher

9b442e5

also make sure all streams are closed Signed-off-by: Keith W. Campbell <[email protected]>

Update comment per reviewer's comment.

eb27b0e

Signed-off-by: Peter Bain <[email protected]>

pdbain-ibm force-pushed the attach_self branch from 747aa97 to eb27b0e Compare September 25, 2017 14:40

pshipton and others added 3 commits September 25, 2017 11:40

Merge pull request eclipse-openj9#120 from DanHeidinga/djh/limit_chec…

3551fdb

…kout Use shallow clone of the openj9-openjdk-jdk9 repo in travis build

Update per reviewer comments

0a70e4d

Changes for OpenJ9 issue eclipse-openj9#126 Signed-off-by: Peter Bain <[email protected]>

DanHeidinga and others added 10 commits September 25, 2017 13:49

Merge pull request eclipse-openj9#19 from babsingh/jcl_tnpe_to_ncdfe

a0c8d18

Translate TypeNotPresentException to NoClassDefFoundError

Replace J9VM and JIT repo references with OpenJ9

ce1c5c8

Signed-off-by: Joe Dekoning <[email protected]>

Remove tooling to restore original attach API code

1e5f75d

During refactoring of the attach API, certain classes were being replaced due to legacy build scripts and needed to be replaced. This is no longer the case. Signed-off-by: Peter Bain <[email protected]>

Merge pull request eclipse-openj9#95 from ncough/eaLoopCheck

c01ae1b

Correct escape analysis loop check

Update jcl project description

08ede0b

update preprocessor builder and nature remove unnecessary builders and natures Signed-off-by: Keith W. Campbell <[email protected]>

Merge pull request eclipse-openj9#134 from keithc-ca/jcl-project-desc…

d68133f

…ription Update jcl project description

Merge pull request eclipse-openj9#130 from pdbain-ibm/attach_class_hack

8ee6b32

Remove tooling to restore original attach API code

Merge pull request eclipse-openj9#96 from jdekonin/master

d2e7c02

Replace J9VM and JIT repo references with OpenJ9

Support receiving parent project warnings-as-errors parameter

92f1d63

Signed-off-by: Ben Walsh <[email protected]>

pdbain-ibm added 3 commits September 27, 2017 08:21

Update comment per reviewer's comment.

9ac5be0

Signed-off-by: Peter Bain <[email protected]>

Update per reviewer comments

bfe80ff

Changes for OpenJ9 issue eclipse-openj9#126 Signed-off-by: Peter Bain <[email protected]>

Merge branch 'attach_self' of github.com:pdbain-ibm/openj9 into attac…

6719fa7

…h_self

pdbain-ibm closed this Sep 27, 2017

pdbain-ibm mentioned this pull request Sep 28, 2017

Disable late attach to self by default #158

Merged

SueChaplain mentioned this pull request Nov 6, 2017

Add detailed instructions for building on Windows #521

Merged

amicic pushed a commit to amicic/openj9 that referenced this pull request Jan 16, 2018

Merge pull request eclipse-openj9#18 from Graham-Chapman/ibm_sdk

04b088f

Compile error fix

myspringchen mentioned this pull request Mar 9, 2018

Should OpenJ9 throw a verifyerror for my test class? #1404

Closed

urisimchoni mentioned this pull request Apr 2, 2018

JIT failure when JVMTI field watches are added #1572

Closed

gacholio mentioned this pull request Jul 30, 2018

cmdLineTester_jvmtitests_hcr_OSRG_nongold_1 Test-extended.functional-JDK10-linux_x86-64 rc004 gpf #2472

Closed

DanHeidinga mentioned this pull request Jun 3, 2019

jdk11: jtreg java/util/concurrent/ArrayBlockingQueue/WhiteBox.java crashes on MacOS #5988

Closed

kusumachalasani mentioned this pull request Oct 14, 2019

Dacapo sub-benchmark "eclipse" crashes when run with large load #7452

Open

manqingl mentioned this pull request Oct 28, 2019

Crash from runCallInMethod() in related to shared classes cache #7611

Closed

hzongaro pushed a commit to hzongaro/openj9 that referenced this pull request Mar 31, 2020

Merge pull request eclipse-openj9#18 from hzongaro/ea-use-env-valuety…

e458cf7

…pe-queries Switch to value type query methods from ClassEnv

mpirvu mentioned this pull request Apr 19, 2021

"Unknown bytecode to JIT 206" assert failure during testJITServer_1 #12468

Closed

manqingl mentioned this pull request Jun 16, 2021

Infinite loop leading to memory corruption and crash. #12979

Closed

a0304 mentioned this pull request Aug 3, 2021

OpenJ9 JVM crash when loading native library in Linux #13269

Open

urisimchoni mentioned this pull request Nov 23, 2021

Crash or OutOfMemoryError when using AsyncGetCallTrace() #13838

Closed

mkarrahcl mentioned this pull request Sep 26, 2023

"dump java core" is hanging process on RHEL 9.1/9.2 using Semeru OpenJDK 17 #18204

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Disable late attach to self by default #18

Disable late attach to self by default #18

pdbain-ibm commented Sep 14, 2017 •

edited by pshipton

Loading

DanHeidinga Sep 15, 2017

pdbain-ibm Sep 19, 2017

DanHeidinga Sep 15, 2017

pdbain-ibm Sep 19, 2017

DanHeidinga Sep 22, 2017

DanHeidinga commented Sep 15, 2017

genie-openj9 commented Sep 19, 2017

genie-openj9 commented Sep 19, 2017

genie-openj9 commented Sep 19, 2017

DanHeidinga Sep 22, 2017

pdbain-ibm Sep 25, 2017

DanHeidinga Sep 22, 2017

DanHeidinga Sep 22, 2017

pdbain-ibm Sep 25, 2017

DanHeidinga Sep 22, 2017

pdbain-ibm Sep 25, 2017

DanHeidinga Sep 22, 2017

pdbain-ibm Sep 25, 2017

DanHeidinga Sep 22, 2017

pdbain-ibm Sep 25, 2017

DanHeidinga Sep 22, 2017

pdbain-ibm Sep 25, 2017

DanHeidinga Sep 25, 2017

pdbain-ibm Sep 25, 2017

DanHeidinga Sep 22, 2017

pdbain-ibm Sep 25, 2017

DanHeidinga Sep 22, 2017

pdbain-ibm Sep 25, 2017

DanHeidinga Sep 22, 2017

pdbain-ibm Sep 25, 2017

pdbain-ibm commented Sep 25, 2017

pdbain-ibm commented Sep 27, 2017

pdbain-ibm commented Sep 27, 2017


		import org.testng.log4testng.Logger;

		/* This is was copied from org.openj9.test.attachAPI.TargetManager in Java8AndUp.

		@@ -0,0 +1,30 @@
		/* Copyright (c) 2015, 2017 IBM Corp. and others

Disable late attach to self by default #18

Disable late attach to self by default #18

Conversation

pdbain-ibm commented Sep 14, 2017 • edited by pshipton Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DanHeidinga commented Sep 15, 2017

genie-openj9 commented Sep 19, 2017

genie-openj9 commented Sep 19, 2017

genie-openj9 commented Sep 19, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pdbain-ibm commented Sep 25, 2017

pdbain-ibm commented Sep 27, 2017

pdbain-ibm commented Sep 27, 2017

pdbain-ibm commented Sep 14, 2017 •

edited by pshipton

Loading