8269542: JDWP: EnableCollection support is no longer spec compliant after JDK-8255987

8258071: Fix for JDK-8255987 can be subverted with ObjectReference.EnableCollection

Reviewed-by: dholmes, pliden

Author: plummercj

src/jdk.jdwp.agent/share/native/libjdwp/commonRef.c

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 1998, 2020, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1998, 2022, 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
@@ -82,22 +82,30 @@ newSeqNum(void)
     return gdata->nextSeqNum++;
 }
 
+/* Returns true if this is a strong reference, meaning that either or both of
+   isPinAll and isCommonPin are true. */
+static jboolean
+isStrong(RefNode* node)
+{
+    return node->isPinAll || node->isCommonPin;
+}
+
 /* Create a fresh RefNode structure, create a weak ref and tag the object */
 static RefNode *
 createNode(JNIEnv *env, jobject ref)
 {
     RefNode   *node;
     jobject    strongOrWeakRef;
     jvmtiError error;
-    jboolean   pin = gdata->pinAllCount != 0;
+    jboolean   pinAll = 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;
     }
 
-    if (pin) {
+    if (pinAll) {
         /* Create strong reference to make sure we have a reference */
         strongOrWeakRef = JNI_FUNC_PTR(env,NewGlobalRef)(env, ref);
     } else {
@@ -116,7 +124,7 @@ createNode(JNIEnv *env, jobject ref)
     error = JVMTI_FUNC_PTR(gdata->jvmti, SetTag)
                           (gdata->jvmti, strongOrWeakRef, ptr_to_jlong(node));
     if ( error != JVMTI_ERROR_NONE ) {
-        if (pin) {
+        if (pinAll) {
             JNI_FUNC_PTR(env,DeleteGlobalRef)(env, strongOrWeakRef);
         } else {
             JNI_FUNC_PTR(env,DeleteWeakGlobalRef)(env, strongOrWeakRef);
@@ -128,7 +136,8 @@ createNode(JNIEnv *env, jobject ref)
     /* Fill in RefNode */
     node->ref         = strongOrWeakRef;
     node->count       = 1;
-    node->strongCount = pin ? 1 : 0;
+    node->isPinAll    = pinAll;
+    node->isCommonPin = JNI_FALSE;
     node->seqNum      = newSeqNum();
 
     /* Count RefNode's created */
@@ -146,7 +155,7 @@ deleteNode(JNIEnv *env, RefNode *node)
         /* Clear tag */
         (void)JVMTI_FUNC_PTR(gdata->jvmti,SetTag)
                             (gdata->jvmti, node->ref, NULL_OBJECT_ID);
-        if (node->strongCount != 0) {
+        if (isStrong(node)) {
             JNI_FUNC_PTR(env,DeleteGlobalRef)(env, node->ref);
         } else {
             JNI_FUNC_PTR(env,DeleteWeakGlobalRef)(env, node->ref);
@@ -158,9 +167,9 @@ deleteNode(JNIEnv *env, RefNode *node)
 
 /* Change a RefNode to have a strong reference */
 static jobject
-strengthenNode(JNIEnv *env, RefNode *node)
+strengthenNode(JNIEnv *env, RefNode *node, jboolean isPinAll)
 {
-    if (node->strongCount == 0) {
+    if (!isStrong(node)) {
         jobject strongRef;
 
         strongRef = JNI_FUNC_PTR(env,NewGlobalRef)(env, node->ref);
@@ -176,20 +185,27 @@ strengthenNode(JNIEnv *env, RefNode *node)
         if (strongRef != NULL) {
             JNI_FUNC_PTR(env,DeleteWeakGlobalRef)(env, node->ref);
             node->ref         = strongRef;
-            node->strongCount = 1;
+        } else {
+          return NULL;
         }
-        return strongRef;
+    }
+    if (isPinAll) {
+        node->isPinAll = JNI_TRUE;
     } else {
-        node->strongCount++;
-        return node->ref;
+        node->isCommonPin = JNI_TRUE;
     }
+    return node->ref;
 }
 
 /* Change a RefNode to have a weak reference */
 static jweak
-weakenNode(JNIEnv *env, RefNode *node)
+weakenNode(JNIEnv *env, RefNode *node, jboolean isUnpinAll)
 {
-    if (node->strongCount == 1) {
+    jboolean willStillBeStrong = (node->isPinAll && !isUnpinAll) || (node->isCommonPin && isUnpinAll);
+
+    // If the node is strong, but the reason(s) for it being strong
+    // will no longer exist, then weaken it.
+    if (isStrong(node) && !willStillBeStrong) {
         jweak weakRef;
 
         weakRef = JNI_FUNC_PTR(env,NewWeakGlobalRef)(env, node->ref);
@@ -200,16 +216,18 @@ weakenNode(JNIEnv *env, RefNode *node)
 
         if (weakRef != NULL) {
             JNI_FUNC_PTR(env,DeleteGlobalRef)(env, node->ref);
-            node->ref         = weakRef;
-            node->strongCount = 0;
+            node->ref      = weakRef;
+        } else {
+          return NULL;
         }
-        return weakRef;
+    }
+
+    if (isUnpinAll) {
+        node->isPinAll = JNI_FALSE;
     } else {
-        if (node->strongCount > 0) {
-            node->strongCount--;
-        }
-        return node->ref;
+        node->isCommonPin = JNI_FALSE;
     }
+    return node->ref;
 }
 
 /*
@@ -470,7 +488,7 @@ commonRef_idToRef(JNIEnv *env, jlong id)
 
         node = findNodeByID(env, id);
         if (node != NULL) {
-            if (node->strongCount != 0) {
+          if (isStrong(node)) {
                 saveGlobalRef(env, node->ref, &ref);
             } else {
                 jobject lref;
@@ -522,7 +540,7 @@ commonRef_pin(jlong id)
         } else {
             jobject strongRef;
 
-            strongRef = strengthenNode(env, node);
+            strongRef = strengthenNode(env, node, JNI_FALSE /* isPinAll */);
             if (strongRef == NULL) {
                 /*
                  * Referent has been collected, clean up now.
@@ -551,7 +569,7 @@ commonRef_unpin(jlong id)
         if (node != NULL) {
             jweak weakRef;
 
-            weakRef = weakenNode(env, node);
+            weakRef = weakenNode(env, node, JNI_FALSE /* isUnpinAll */);
             if (weakRef == NULL) {
                 error = AGENT_ERROR_OUT_OF_MEMORY;
             }
@@ -585,7 +603,7 @@ commonRef_pinAll()
                 while (node != NULL) {
                     jobject strongRef;
 
-                    strongRef = strengthenNode(env, node);
+                    strongRef = strengthenNode(env, node, JNI_TRUE /* isPinAll */);
 
                     /* Has the object been collected? */
                     if (strongRef == NULL) {
@@ -628,7 +646,7 @@ commonRef_unpinAll()
                 for (node = gdata->objectsByID[i]; node != NULL; node = node->next) {
                     jweak weakRef;
 
-                    weakRef = weakenNode(env, node);
+                    weakRef = weakenNode(env, node, JNI_TRUE /* isUnpinAll */);
                     if (weakRef == NULL) {
                         EXIT_ERROR(AGENT_ERROR_NULL_POINTER,"NewWeakGlobalRef");
                     }
@@ -676,8 +694,7 @@ commonRef_compact(void)
                 prev = NULL;
                 while (node != NULL) {
                     /* Has the object been collected? */
-                    if ( (node->strongCount == 0) &&
-                          isSameObject(env, node->ref, NULL)) {
+                  if (!isStrong(node) && isSameObject(env, node->ref, NULL)) {
                         RefNode *freed;
 
                         /* Detach from the ID list */

src/jdk.jdwp.agent/share/native/libjdwp/util.h

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 1998, 2021, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1998, 2022, 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
@@ -65,7 +65,8 @@ typedef struct RefNode {
     jobject      ref;           /* could be strong or weak */
     struct RefNode *next;       /* next RefNode* in bucket chain */
     jint         count;         /* count of references */
-    unsigned     strongCount;   /* count of strong reference */
+    jboolean     isPinAll;      /* true if this is a strong reference due to a commonRef_pinAll() */
+    jboolean     isCommonPin;   /* true if this is a strong reference due to a commonRef_pin() */
 } RefNode;
 
 /* Value of a NULL ID */