Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
8255987: JDI tests fail with com.sun.jdi.ObjectCollectedException
Reviewed-by: dholmes, cjplummer
  • Loading branch information
pliden committed Dec 9, 2020
1 parent 9ce3d80 commit 79f1dfb
Show file tree
Hide file tree
Showing 8 changed files with 168 additions and 33 deletions.
144 changes: 118 additions & 26 deletions src/jdk.jdwp.agent/share/native/libjdwp/commonRef.c
@@ -1,5 +1,5 @@
/*
* Copyright (c) 1998, 2013, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 1998, 2020, Oracle and/or its affiliates. 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
Expand Down Expand Up @@ -87,38 +87,49 @@ static RefNode *
createNode(JNIEnv *env, jobject ref)
{
RefNode *node;
jobject weakRef;
jobject strongOrWeakRef;
jvmtiError error;
jboolean pin = gdata->pinAllCount != 0;

/* Could allocate RefNode's in blocks, not sure it would help much */
node = (RefNode*)jvmtiAllocate((int)sizeof(RefNode));
if (node == NULL) {
return NULL;
}

/* Create weak reference to make sure we have a reference */
weakRef = JNI_FUNC_PTR(env,NewWeakGlobalRef)(env, ref);
// NewWeakGlobalRef can throw OOM, clear exception here.
if ((*env)->ExceptionCheck(env)) {
(*env)->ExceptionClear(env);
jvmtiDeallocate(node);
return NULL;
if (pin) {
/* Create strong reference to make sure we have a reference */
strongOrWeakRef = JNI_FUNC_PTR(env,NewGlobalRef)(env, ref);
} else {
/* Create weak reference to make sure we have a reference */
strongOrWeakRef = JNI_FUNC_PTR(env,NewWeakGlobalRef)(env, ref);

// NewWeakGlobalRef can throw OOM, clear exception here.
if ((*env)->ExceptionCheck(env)) {
(*env)->ExceptionClear(env);
jvmtiDeallocate(node);
return NULL;
}
}

/* Set tag on weakRef */
/* Set tag on strongOrWeakRef */
error = JVMTI_FUNC_PTR(gdata->jvmti, SetTag)
(gdata->jvmti, weakRef, ptr_to_jlong(node));
(gdata->jvmti, strongOrWeakRef, ptr_to_jlong(node));
if ( error != JVMTI_ERROR_NONE ) {
JNI_FUNC_PTR(env,DeleteWeakGlobalRef)(env, weakRef);
if (pin) {
JNI_FUNC_PTR(env,DeleteGlobalRef)(env, strongOrWeakRef);
} else {
JNI_FUNC_PTR(env,DeleteWeakGlobalRef)(env, strongOrWeakRef);
}
jvmtiDeallocate(node);
return NULL;
}

/* Fill in RefNode */
node->ref = weakRef;
node->isStrong = JNI_FALSE;
node->count = 1;
node->seqNum = newSeqNum();
node->ref = strongOrWeakRef;
node->count = 1;
node->strongCount = pin ? 1 : 0;
node->seqNum = newSeqNum();

/* Count RefNode's created */
gdata->objectsByIDcount++;
Expand All @@ -135,7 +146,7 @@ deleteNode(JNIEnv *env, RefNode *node)
/* Clear tag */
(void)JVMTI_FUNC_PTR(gdata->jvmti,SetTag)
(gdata->jvmti, node->ref, NULL_OBJECT_ID);
if (node->isStrong) {
if (node->strongCount != 0) {
JNI_FUNC_PTR(env,DeleteGlobalRef)(env, node->ref);
} else {
JNI_FUNC_PTR(env,DeleteWeakGlobalRef)(env, node->ref);
Expand All @@ -149,7 +160,7 @@ deleteNode(JNIEnv *env, RefNode *node)
static jobject
strengthenNode(JNIEnv *env, RefNode *node)
{
if (!node->isStrong) {
if (node->strongCount == 0) {
jobject strongRef;

strongRef = JNI_FUNC_PTR(env,NewGlobalRef)(env, node->ref);
Expand All @@ -164,11 +175,12 @@ strengthenNode(JNIEnv *env, RefNode *node)
}
if (strongRef != NULL) {
JNI_FUNC_PTR(env,DeleteWeakGlobalRef)(env, node->ref);
node->ref = strongRef;
node->isStrong = JNI_TRUE;
node->ref = strongRef;
node->strongCount = 1;
}
return strongRef;
} else {
node->strongCount++;
return node->ref;
}
}
Expand All @@ -177,7 +189,7 @@ strengthenNode(JNIEnv *env, RefNode *node)
static jweak
weakenNode(JNIEnv *env, RefNode *node)
{
if (node->isStrong) {
if (node->strongCount == 1) {
jweak weakRef;

weakRef = JNI_FUNC_PTR(env,NewWeakGlobalRef)(env, node->ref);
Expand All @@ -188,11 +200,12 @@ weakenNode(JNIEnv *env, RefNode *node)

if (weakRef != NULL) {
JNI_FUNC_PTR(env,DeleteGlobalRef)(env, node->ref);
node->ref = weakRef;
node->isStrong = JNI_FALSE;
node->ref = weakRef;
node->strongCount = 0;
}
return weakRef;
} else {
node->strongCount--;
return node->ref;
}
}
Expand Down Expand Up @@ -372,7 +385,8 @@ void
commonRef_initialize(void)
{
gdata->refLock = debugMonitorCreate("JDWP Reference Table Monitor");
gdata->nextSeqNum = 1; /* 0 used for error indication */
gdata->nextSeqNum = 1; /* 0 used for error indication */
gdata->pinAllCount = 0;
initializeObjectsByID(HASH_INIT_SIZE);
}

Expand Down Expand Up @@ -454,7 +468,7 @@ commonRef_idToRef(JNIEnv *env, jlong id)

node = findNodeByID(env, id);
if (node != NULL) {
if (node->isStrong) {
if (node->strongCount != 0) {
saveGlobalRef(env, node->ref, &ref);
} else {
jobject lref;
Expand Down Expand Up @@ -544,6 +558,84 @@ commonRef_unpin(jlong id)
return error;
}

/* Prevent garbage collection of object */
void
commonRef_pinAll()
{
debugMonitorEnter(gdata->refLock); {
gdata->pinAllCount++;

if (gdata->pinAllCount == 1) {
JNIEnv *env;
RefNode *node;
RefNode *prev;
int i;

env = getEnv();

/*
* Walk through the id-based hash table. Detach any nodes
* for which the ref has been collected.
*/
for (i = 0; i < gdata->objectsByIDsize; i++) {
node = gdata->objectsByID[i];
prev = NULL;
while (node != NULL) {
jobject strongRef;

strongRef = strengthenNode(env, node);

/* Has the object been collected? */
if (strongRef == NULL) {
RefNode *freed;

/* Detach from the ID list */
if (prev == NULL) {
gdata->objectsByID[i] = node->next;
} else {
prev->next = node->next;
}
freed = node;
node = node->next;
deleteNode(env, freed);
} else {
prev = node;
node = node->next;
}
}
}
}
} debugMonitorExit(gdata->refLock);
}

/* Permit garbage collection of objects */
void
commonRef_unpinAll()
{
debugMonitorEnter(gdata->refLock); {
gdata->pinAllCount--;

if (gdata->pinAllCount == 0) {
JNIEnv *env;
RefNode *node;
int i;

env = getEnv();

for (i = 0; i < gdata->objectsByIDsize; i++) {
for (node = gdata->objectsByID[i]; node != NULL; node = node->next) {
jweak weakRef;

weakRef = weakenNode(env, node);
if (weakRef == NULL) {
EXIT_ERROR(AGENT_ERROR_NULL_POINTER,"NewWeakGlobalRef");
}
}
}
}
} debugMonitorExit(gdata->refLock);
}

/* Release tracking of an object by ID */
void
commonRef_release(JNIEnv *env, jlong id)
Expand Down Expand Up @@ -582,7 +674,7 @@ commonRef_compact(void)
prev = NULL;
while (node != NULL) {
/* Has the object been collected? */
if ( (!node->isStrong) &&
if ( (node->strongCount == 0) &&
isSameObject(env, node->ref, NULL)) {
RefNode *freed;

Expand Down
4 changes: 3 additions & 1 deletion src/jdk.jdwp.agent/share/native/libjdwp/commonRef.h
@@ -1,5 +1,5 @@
/*
* Copyright (c) 1998, 2005, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 1998, 2020, Oracle and/or its affiliates. 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
Expand Down Expand Up @@ -34,6 +34,8 @@ jobject commonRef_idToRef(JNIEnv *env, jlong id);
void commonRef_idToRef_delete(JNIEnv *env, jobject ref);
jvmtiError commonRef_pin(jlong id);
jvmtiError commonRef_unpin(jlong id);
void commonRef_pinAll();
void commonRef_unpinAll();
void commonRef_releaseMultiple(JNIEnv *env, jlong id, jint refCount);
void commonRef_release(JNIEnv *env, jlong id);
void commonRef_compact(void);
Expand Down
11 changes: 11 additions & 0 deletions src/jdk.jdwp.agent/share/native/libjdwp/threadControl.c
Expand Up @@ -1553,6 +1553,12 @@ threadControl_suspendAll(void)
}

if (error == JVMTI_ERROR_NONE) {
/*
* Pin all objects to prevent objects from being
* garbage collected while the VM is suspended.
*/
commonRef_pinAll();

suspendAllCount++;
}

Expand Down Expand Up @@ -1604,6 +1610,11 @@ threadControl_resumeAll(void)
}

if (suspendAllCount > 0) {
/*
* Unpin all objects.
*/
commonRef_unpinAll();

suspendAllCount--;
}

Expand Down
3 changes: 2 additions & 1 deletion src/jdk.jdwp.agent/share/native/libjdwp/util.h
Expand Up @@ -65,7 +65,7 @@ typedef struct RefNode {
jobject ref; /* could be strong or weak */
struct RefNode *next; /* next RefNode* in bucket chain */
jint count; /* count of references */
unsigned isStrong : 1; /* 1 means this is a string reference */
unsigned strongCount; /* count of strong reference */
} RefNode;

/* Value of a NULL ID */
Expand Down Expand Up @@ -128,6 +128,7 @@ typedef struct {
/* Common References static data */
jrawMonitorID refLock;
jlong nextSeqNum;
unsigned pinAllCount;
RefNode **objectsByID;
int objectsByIDsize;
int objectsByIDcount;
Expand Down
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2001, 2018, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2001, 2020, Oracle and/or its affiliates. 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
Expand Down Expand Up @@ -144,9 +144,23 @@ private int runThis (String argv[], PrintStream out) {
log1(" TESTING BEGINS");

for (int i = 0; ; i++) {
pipe.println("newcheck");
pipe.println("newcheck");

// There are potentially other non-test Java threads allocating objects and triggering
// GC's so we need to suspend the target VM to avoid the objects created in the test
// from being accidentally GC'ed. However, we need the target VM temporary resumed
// while reading its response. Below we resume the target VM (if required) and suspend
// it only after pipe.readln() returns.

// On the first iteration the target VM is not suspended yet.
if (i > 0) {
debuggee.resume();
}
line = pipe.readln();

// Suspending target VM to prevent other non-test Java threads from triggering GCs.
debuggee.suspend();

if (line.equals("checkend")) {
log2(" : returned string is 'checkend'");
break ;
Expand Down Expand Up @@ -230,6 +244,7 @@ private int runThis (String argv[], PrintStream out) {
//-------------------------------------------------- test summary section
//------------------------------------------------- standard end section

debuggee.resume();
pipe.println("quit");
log2("waiting for the debuggee to finish ...");
debuggee.waitFor();
Expand Down
Expand Up @@ -189,7 +189,9 @@ public void testClassType(String className) {
objectReferences.add(classType.newInstance(breakpointEvent.thread(), method, new ArrayList<Value>(), 0));
}

debuggee.resume();
checkDebugeeAnswer_instances(className, baseInstances);
debuggee.suspend();

break;
}
Expand Down
Expand Up @@ -79,7 +79,14 @@ public void doTest() {
// create array in debuggee VM till VMOutOfMemoryException
while (true) {
ArrayReference array = referenceType.newInstance(100000);
array.disableCollection();
try {
// Since the VM is not suspended, the object may have been collected
// before disableCollection() could be called on it. Just ignore and
// continue doing allocations until we run out of memory.
array.disableCollection();
} catch (ObjectCollectedException e) {
continue;
}
objects.add(array);
}
} catch (VMOutOfMemoryException e) {
Expand Down
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2007, 2018, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2007, 2020, Oracle and/or its affiliates. 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
Expand Down Expand Up @@ -67,10 +67,15 @@ public boolean parseCommand(String command) {
return false;
}

// Keep class loader alive to avoid ObjectCollectedException
// on the debugger side, in case the GC unloads the class and
// invalidates code locations.
private TestClassLoader classLoader;

// create instance of given class and execute all methods which names start
// with 'sde_testMethod'
private void executeTestMethods(String className) {
TestClassLoader classLoader = new TestClassLoader();
classLoader = new TestClassLoader();
classLoader.setClassPath(classpath);

try {
Expand Down

1 comment on commit 79f1dfb

@openjdk-notifier
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please sign in to comment.