6572160: 3/3 Instrumentation.getObjectSize triggers JVM crash in JPLISAssert in shutdown
Summary: Tolerate JVMTI_ERROR_WRONG_PHASE return codes so that JLI methods can be called to the end of VM's life.
Reviewed-by: ohair, sspitsyn
--- a/jdk/src/share/instrument/InvocationAdapter.c Mon Mar 24 16:59:07 2008 -0700
+++ b/jdk/src/share/instrument/InvocationAdapter.c Mon Mar 24 17:12:01 2008 -0700
@@ -626,6 +626,7 @@
jvmtiError jvmtierr;
jvmtierr = (*jvmtienv)->AddToSystemClassLoaderSearch(jvmtienv, jarfile);
+ check_phase_ret_1(jvmtierr);
if (jvmtierr == JVMTI_ERROR_NONE) {
return 0;
@@ -634,6 +635,7 @@
jvmtiError err;
err = (*jvmtienv)->GetPhase(jvmtienv, &phase);
+ /* can be called from any phase */
jplis_assert(err == JVMTI_ERROR_NONE);
if (phase == JVMTI_PHASE_LIVE) {
@@ -805,6 +807,8 @@
/* print warning if boot class path not updated */
if (jvmtierr != JVMTI_ERROR_NONE) {
+ check_phase_blob_ret(jvmtierr, free(path));
+
fprintf(stderr, "WARNING: %s not added to bootstrap class loader search: ", path);
switch (jvmtierr) {
case JVMTI_ERROR_ILLEGAL_ARGUMENT :
--- a/jdk/src/share/instrument/JPLISAgent.c Mon Mar 24 16:59:07 2008 -0700
+++ b/jdk/src/share/instrument/JPLISAgent.c Mon Mar 24 17:12:01 2008 -0700
@@ -179,6 +179,7 @@
jvmtierror = (*jvmtienv)->GetEnvironmentLocalStorage(
jvmtienv,
(void**)&environment);
+ /* can be called from any phase */
jplis_assert(jvmtierror == JVMTI_ERROR_NONE);
if (jvmtierror == JVMTI_ERROR_NONE) {
@@ -230,6 +231,7 @@
/* don't leak envs */
if ( initerror != JPLIS_INIT_ERROR_NONE ) {
jvmtiError jvmtierror = (*jvmtienv)->DisposeEnvironment(jvmtienv);
+ /* can be called from any phase */
jplis_assert(jvmtierror == JVMTI_ERROR_NONE);
}
}
@@ -277,6 +279,7 @@
jvmtierror = (*jvmtienv)->SetEnvironmentLocalStorage(
jvmtienv,
&(agent->mNormalEnvironment));
+ /* can be called from any phase */
jplis_assert(jvmtierror == JVMTI_ERROR_NONE);
/* check what capabilities are available */
@@ -284,11 +287,17 @@
/* check phase - if live phase then we don't need the VMInit event */
jvmtierror = (*jvmtienv)->GetPhase(jvmtienv, &phase);
+ /* can be called from any phase */
jplis_assert(jvmtierror == JVMTI_ERROR_NONE);
if (phase == JVMTI_PHASE_LIVE) {
return JPLIS_INIT_ERROR_NONE;
}
+ if (phase != JVMTI_PHASE_ONLOAD) {
+ /* called too early or called too late; either way bail out */
+ return JPLIS_INIT_ERROR_FAILURE;
+ }
+
/* now turn on the VMInit event */
if ( jvmtierror == JVMTI_ERROR_NONE ) {
jvmtiEventCallbacks callbacks;
@@ -298,6 +307,7 @@
jvmtierror = (*jvmtienv)->SetEventCallbacks( jvmtienv,
&callbacks,
sizeof(callbacks));
+ check_phase_ret_blob(jvmtierror, JPLIS_INIT_ERROR_FAILURE);
jplis_assert(jvmtierror == JVMTI_ERROR_NONE);
}
@@ -307,6 +317,7 @@
JVMTI_ENABLE,
JVMTI_EVENT_VM_INIT,
NULL /* all threads */);
+ check_phase_ret_blob(jvmtierror, JPLIS_INIT_ERROR_FAILURE);
jplis_assert(jvmtierror == JVMTI_ERROR_NONE);
}
@@ -622,6 +633,7 @@
jvmtierror = (*jvmtienv)->SetEventCallbacks( jvmtienv,
&callbacks,
sizeof(callbacks));
+ check_phase_ret_false(jvmtierror);
jplis_assert(jvmtierror == JVMTI_ERROR_NONE);
@@ -632,6 +644,7 @@
JVMTI_DISABLE,
JVMTI_EVENT_VM_INIT,
NULL /* all threads */);
+ check_phase_ret_false(jvmtierror);
jplis_assert(jvmtierror == JVMTI_ERROR_NONE);
}
@@ -642,6 +655,7 @@
JVMTI_ENABLE,
JVMTI_EVENT_CLASS_FILE_LOAD_HOOK,
NULL /* all threads */);
+ check_phase_ret_false(jvmtierror);
jplis_assert(jvmtierror == JVMTI_ERROR_NONE);
}
@@ -660,6 +674,7 @@
memset(&potentialCapabilities, 0, sizeof(potentialCapabilities));
jvmtierror = (*jvmtienv)->GetPotentialCapabilities(jvmtienv, &potentialCapabilities);
+ check_phase_ret(jvmtierror);
jplis_assert(jvmtierror == JVMTI_ERROR_NONE);
if ( jvmtierror == JVMTI_ERROR_NONE ) {
@@ -681,9 +696,11 @@
jvmtiError jvmtierror;
jvmtierror = (*jvmtienv)->GetCapabilities(jvmtienv, &desiredCapabilities);
+ /* can be called from any phase */
jplis_assert(jvmtierror == JVMTI_ERROR_NONE);
desiredCapabilities.can_set_native_method_prefix = 1;
jvmtierror = (*jvmtienv)->AddCapabilities(jvmtienv, &desiredCapabilities);
+ check_phase_ret(jvmtierror);
jplis_assert(jvmtierror == JVMTI_ERROR_NONE);
}
@@ -715,9 +732,11 @@
jvmtiError jvmtierror;
jvmtierror = (*jvmtienv)->GetCapabilities(jvmtienv, &desiredCapabilities);
+ /* can be called from any phase */
jplis_assert(jvmtierror == JVMTI_ERROR_NONE);
desiredCapabilities.can_maintain_original_method_order = 1;
jvmtierror = (*jvmtienv)->AddCapabilities(jvmtienv, &desiredCapabilities);
+ check_phase_ret(jvmtierror);
jplis_assert(jvmtierror == JVMTI_ERROR_NONE);
}
@@ -732,9 +751,11 @@
if (agent->mRedefineAvailable && !agent->mRedefineAdded) {
jvmtierror = (*jvmtienv)->GetCapabilities(jvmtienv, &desiredCapabilities);
+ /* can be called from any phase */
jplis_assert(jvmtierror == JVMTI_ERROR_NONE);
desiredCapabilities.can_redefine_classes = 1;
jvmtierror = (*jvmtienv)->AddCapabilities(jvmtienv, &desiredCapabilities);
+ check_phase_ret(jvmtierror);
/*
* With mixed premain/agentmain agents then it's possible that the
@@ -1026,6 +1047,7 @@
jvmtierror = (*jvmtienv)->IsModifiableClass( jvmtienv,
clazz,
&is_modifiable);
+ check_phase_ret_false(jvmtierror);
jplis_assert(jvmtierror == JVMTI_ERROR_NONE);
return is_modifiable;
@@ -1231,6 +1253,7 @@
if (!errorOccurred) {
jvmtiError errorCode = JVMTI_ERROR_NONE;
errorCode = (*jvmtienv)->RedefineClasses(jvmtienv, numDefs, classDefs);
+ check_phase_blob_ret(errorCode, deallocate(jvmtienv, (void*)classDefs));
errorOccurred = (errorCode != JVMTI_ERROR_NONE);
if ( errorOccurred ) {
createAndThrowThrowableFromJVMTIErrorCode(jnienv, errorCode);
@@ -1264,6 +1287,7 @@
classLoader,
&classCount,
&classes);
+ check_phase_ret_blob(jvmtierror, localArray);
errorOccurred = (jvmtierror != JVMTI_ERROR_NONE);
jplis_assert(!errorOccurred);
@@ -1325,6 +1349,7 @@
jvmtiError jvmtierror = JVMTI_ERROR_NONE;
jvmtierror = (*jvmtienv)->GetObjectSize(jvmtienv, objectToSize, &objectSize);
+ check_phase_ret_0(jvmtierror);
jplis_assert(jvmtierror == JVMTI_ERROR_NONE);
if ( jvmtierror != JVMTI_ERROR_NONE ) {
createAndThrowThrowableFromJVMTIErrorCode(jnienv, jvmtierror);
@@ -1374,6 +1399,7 @@
} else {
jvmtierror = (*jvmtienv)->AddToSystemClassLoaderSearch(jvmtienv, platformChars);
}
+ check_phase_ret(jvmtierror);
if ( jvmtierror != JVMTI_ERROR_NONE ) {
createAndThrowThrowableFromJVMTIErrorCode(jnienv, jvmtierror);
@@ -1464,6 +1490,7 @@
}
err = (*jvmtienv)->SetNativeMethodPrefixes(jvmtienv, inx, (char**)prefixes);
+ /* can be called from any phase */
jplis_assert(err == JVMTI_ERROR_NONE);
for (i = 0; i < inx; i++) {
--- a/jdk/src/share/instrument/JPLISAgent.h Mon Mar 24 16:59:07 2008 -0700
+++ b/jdk/src/share/instrument/JPLISAgent.h Mon Mar 24 17:12:01 2008 -0700
@@ -266,6 +266,48 @@
#define jvmti(a) a->mNormalEnvironment.mJVMTIEnv
+/*
+ * A set of macros for insulating the JLI method callers from
+ * JVMTI_ERROR_WRONG_PHASE return codes.
+ */
+
+/* for a JLI method where "blob" is executed before simply returning */
+#define check_phase_blob_ret(ret, blob) \
+ if ((ret) == JVMTI_ERROR_WRONG_PHASE) { \
+ blob; \
+ return; \
+ }
+
+/* for a JLI method where simply returning is benign */
+#define check_phase_ret(ret) \
+ if ((ret) == JVMTI_ERROR_WRONG_PHASE) { \
+ return; \
+ }
+
+/* for a JLI method where returning zero (0) is benign */
+#define check_phase_ret_0(ret) \
+ if ((ret) == JVMTI_ERROR_WRONG_PHASE) { \
+ return 0; \
+ }
+
+/* for a JLI method where returning one (1) is benign */
+#define check_phase_ret_1(ret) \
+ if ((ret) == JVMTI_ERROR_WRONG_PHASE) { \
+ return 1; \
+ }
+
+/* for a case where a specific "blob" must be returned */
+#define check_phase_ret_blob(ret, blob) \
+ if ((ret) == JVMTI_ERROR_WRONG_PHASE) { \
+ return (blob); \
+ }
+
+/* for a JLI method where returning false is benign */
+#define check_phase_ret_false(ret) \
+ if ((ret) == JVMTI_ERROR_WRONG_PHASE) { \
+ return (jboolean) 0; \
+ }
+
#ifdef __cplusplus
} /* extern "C" */
#endif /* __cplusplus */
--- a/jdk/src/share/instrument/Reentrancy.c Mon Mar 24 16:59:07 2008 -0700
+++ b/jdk/src/share/instrument/Reentrancy.c Mon Mar 24 17:12:01 2008 -0700
@@ -74,6 +74,7 @@
jvmtienv,
thread,
newValue);
+ check_phase_ret_blob(error, error);
#if JPLISASSERT_ENABLEASSERTIONS
assertTLSValue( jvmtienv,
@@ -96,6 +97,7 @@
jvmtienv,
thread,
&test);
+ check_phase_ret(error);
jplis_assert(error == JVMTI_ERROR_NONE);
jplis_assert(test == expected);
}
@@ -111,6 +113,7 @@
jvmtienv,
thread,
&storedValue);
+ check_phase_ret_false(error);
jplis_assert(error == JVMTI_ERROR_NONE);
if ( error == JVMTI_ERROR_NONE ) {
/* if this thread is already inside, just return false and short-circuit */
--- a/jdk/src/share/instrument/Utilities.c Mon Mar 24 16:59:07 2008 -0700
+++ b/jdk/src/share/instrument/Utilities.c Mon Mar 24 17:12:01 2008 -0700
@@ -46,6 +46,7 @@
error = (*jvmtienv)->Allocate(jvmtienv,
bytecount,
(unsigned char**) &resultBuffer);
+ /* may be called from any phase */
jplis_assert(error == JVMTI_ERROR_NONE);
if ( error != JVMTI_ERROR_NONE ) {
resultBuffer = NULL;
@@ -66,6 +67,7 @@
error = (*jvmtienv)->Deallocate(jvmtienv,
(unsigned char*)buffer);
+ /* may be called from any phase */
jplis_assert_msg(error == JVMTI_ERROR_NONE, "Can't deallocate memory");
return;
}
--- /dev/null Thu Jan 01 00:00:00 1970 +0000
+++ b/jdk/test/java/lang/instrument/StressGetObjectSizeApp.java Mon Mar 24 17:12:01 2008 -0700
@@ -0,0 +1,86 @@
+/*
+ * Copyright 2008 Sun Microsystems, Inc. All Rights Reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Sun Microsystems, Inc., 4150 Network Circle, Santa Clara,
+ * CA 95054 USA or visit www.sun.com if you need additional information or
+ * have any questions.
+ */
+
+import java.lang.instrument.Instrumentation;
+
+public class StressGetObjectSizeApp
+ extends ASimpleInstrumentationTestCase
+{
+
+ /**
+ * Constructor for StressGetObjectSizeApp.
+ * @param name
+ */
+ public StressGetObjectSizeApp(String name)
+ {
+ super(name);
+ }
+
+ public static void
+ main (String[] args)
+ throws Throwable {
+ ATestCaseScaffold test = new StressGetObjectSizeApp(args[0]);
+ test.runTest();
+ }
+
+ protected final void
+ doRunTest()
+ throws Throwable {
+ stressGetObjectSize();
+ }
+
+ public void stressGetObjectSize() {
+ System.out.println("main: an object size=" +
+ fInst.getObjectSize(new Object()));
+
+ RoundAndRound[] threads = new RoundAndRound[10];
+ for (int i = 0; i < threads.length; ++i) {
+ threads[i] = new RoundAndRound(fInst);
+ threads[i].start();
+ }
+ try {
+ Thread.sleep(500); // let all threads get going in their loops
+ } catch (InterruptedException ie) {
+ }
+ System.out.println("stressGetObjectSize: returning");
+ return;
+ }
+
+ private static class RoundAndRound extends Thread {
+ private final Instrumentation inst;
+ private final Object anObject;
+
+ public RoundAndRound(Instrumentation inst) {
+ this.inst = inst;
+ this.anObject = new Object();
+ setDaemon(true);
+ }
+
+ public void run() {
+ long sum = 0;
+ while (true) {
+ sum += inst.getObjectSize(anObject);
+ }
+ }
+ }
+}
--- /dev/null Thu Jan 01 00:00:00 1970 +0000
+++ b/jdk/test/java/lang/instrument/StressGetObjectSizeTest.sh Mon Mar 24 17:12:01 2008 -0700
@@ -0,0 +1,70 @@
+#
+# Copyright 2008 Sun Microsystems, Inc. All Rights Reserved.
+# DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+#
+# This code is free software; you can redistribute it and/or modify it
+# under the terms of the GNU General Public License version 2 only, as
+# published by the Free Software Foundation.
+#
+# This code is distributed in the hope that it will be useful, but WITHOUT
+# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+# FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
+# version 2 for more details (a copy is included in the LICENSE file that
+# accompanied this code).
+#
+# You should have received a copy of the GNU General Public License version
+# 2 along with this work; if not, write to the Free Software Foundation,
+# Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+#
+# Please contact Sun Microsystems, Inc., 4150 Network Circle, Santa Clara,
+# CA 95054 USA or visit www.sun.com if you need additional information or
+# have any questions.
+#
+
+# @test
+# @bug 6572160
+# @summary stress getObjectSize() API
+# @author Daniel D. Daugherty as modified from the code of fischman@google.com
+#
+# @run build StressGetObjectSizeApp
+# @run shell MakeJAR.sh basicAgent
+# @run shell StressGetObjectSizeTest.sh
+#
+
+if [ "${TESTJAVA}" = "" ]
+then
+ echo "TESTJAVA not set. Test cannot execute. Failed."
+ exit 1
+fi
+
+if [ "${TESTSRC}" = "" ]
+then
+ echo "TESTSRC not set. Test cannot execute. Failed."
+ exit 1
+fi
+
+if [ "${TESTCLASSES}" = "" ]
+then
+ echo "TESTCLASSES not set. Test cannot execute. Failed."
+ exit 1
+fi
+
+JAVA="${TESTJAVA}"/bin/java
+
+"${JAVA}" ${TESTVMOPTS} -javaagent:basicAgent.jar \
+ -classpath "${TESTCLASSES}" StressGetObjectSizeApp StressGetObjectSizeApp \
+ > output.log 2>&1
+cat output.log
+
+MESG="ASSERTION FAILED"
+grep "$MESG" output.log
+result=$?
+if [ "$result" = 0 ]; then
+ echo "FAIL: found '$MESG' in the test output"
+ result=1
+else
+ echo "PASS: did NOT find '$MESG' in the test output"
+ result=0
+fi
+
+exit $result