# HG changeset patch # User dcubed # Date 1206403921 25200 # Node ID bca3e5a71df17714abc6159bb6c815a298ec7680 # Parent 3da74b83e387a2be7f6b35e65ba9d3a4b1efe52e 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 diff -r 3da74b83e387 -r bca3e5a71df1 jdk/src/share/instrument/InvocationAdapter.c --- 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 : diff -r 3da74b83e387 -r bca3e5a71df1 jdk/src/share/instrument/JPLISAgent.c --- 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++) { diff -r 3da74b83e387 -r bca3e5a71df1 jdk/src/share/instrument/JPLISAgent.h --- 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 */ diff -r 3da74b83e387 -r bca3e5a71df1 jdk/src/share/instrument/Reentrancy.c --- 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 */ diff -r 3da74b83e387 -r bca3e5a71df1 jdk/src/share/instrument/Utilities.c --- 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; } diff -r 3da74b83e387 -r bca3e5a71df1 jdk/test/java/lang/instrument/StressGetObjectSizeApp.java --- /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); + } + } + } +} diff -r 3da74b83e387 -r bca3e5a71df1 jdk/test/java/lang/instrument/StressGetObjectSizeTest.sh --- /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