RFR: 8269542: JDWP: EnableCollection support is no longer spec compliant after JDK-8255987 [v2]

[Chris Plummer]

On Wed, 19 Jan 2022 22:23:03 GMT, David Holmes <dholmes at openjdk.org> wrote:

>> Chris Plummer has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Get rid of isStrong flag and instead just compute it based on the value of isPinAll and isCommonPin. Add comments for some boolean args.
>
> src/jdk.jdwp.agent/share/native/libjdwp/commonRef.c line 181:
> 
>> 179:             node->strongCount = 1;
>> 180:         }
>> 181:         return strongRef;
> 
> Just to be clear about the removal of this line, at this point if `strongRef==NULL` then `node->ref==NULL` (otherwise we would have taken the EXIT_ERROR path) - right?

I believe `node->ref` would be a weak reference to a NULL object, and not itself be NULL. That is why the !isSameObject() call is used rather than just comparing to NULL. It's an error if the global ref allocation fails and `node->ref` does not reference a NULL (collected) object.

However, there does appear to be a bug here. If the allocation failed because the object was collected, we want to return NULL in that case, not node->ref, which is not NULL but references a NULL object. Also, I it's a bug to set isPinAll or isCommonPin when the allocation fails for this reason.

> src/jdk.jdwp.agent/share/native/libjdwp/commonRef.c line 206:
> 
>> 204:             node->strongCount = 0;
>> 205:         }
>> 206:         return weakRef;
> 
> In this case I don't see that the removal of this return is correct if `weakRef==NULL` ?

Similar issue here. It should be returning NULL in this case, not `node->ref.`

-------------

PR: https://git.openjdk.java.net/jdk/pull/7134