URGENT RFR 8153673 [BACKOUT] JDWP: Memory Leak: GlobalRefs never deleted when processing invokeMethod command

[Daniel D. Daugherty]

Thanks!

Dan

On 4/6/16 5:43 PM, Jesper Wilhelmsson wrote:
> Looks good!
> /Jesper
>
> Den 7/4/16 kl. 01:34, skrev Daniel D. Daugherty:
>> Resending with Jesper and Serguei on directly...
>>
>> Greetings,
>>
>> It appears that the fix for this bug:
>>
>>      JDK-4858370 JDWP: Memory Leak: GlobalRefs never deleted when 
>> processing
>>                  invokeMethod command
>>      https://bugs.openjdk.java.net/browse/JDK-4858370
>>
>> has been causing intermittent test failures in the JDK9-hs-rt nightly.
>>
>>
>> ************************************************************
>> I need reviews from Jesper Wilhelmsson and Serguei Spitsyn.
>> ************************************************************
>>
>>
>> This bug is being using to backout JDK-4858370:
>>
>>      JDK-8153673 [BACKOUT] JDWP: Memory Leak: GlobalRefs never 
>> deleted when
>>                  processing invokeMethod command
>>      https://bugs.openjdk.java.net/browse/JDK-8153673
>>
>> Here is the backout webrev URL:
>>
>> http://cr.openjdk.java.net/~dcubed/8153673-webrev/0-jdk9-hs-rt-jdk/
>>
>> Here are the changeset link for JDK-4858370:
>>
>>      http://hg.openjdk.java.net/jdk9/hs-rt/jdk/rev/277d7584fa03
>>
>> Jesper and Serguei, thanks, in advance, for the reviews!
>>
>> Gory details below...
>>
>> Dan
>>
>>
>>
>> Here's my jdk repo sanity check note:
>>
>>   Daniel Daugherty added a comment - 1 hour ago
>> Sanity checks for backing out this fix and its test:
>>
>> $ hg log -r 277d7584fa03
>> changeset: 13880:277d7584fa03
>> parent: 13877:645a9be6eddb
>> user: sgehwolf
>> date: Mon Mar 21 11:24:09 2016 +0100
>> summary: 4858370: JDWP: Memory Leak: GlobalRefs never deleted when 
>> processing
>> invokeMethod command
>>
>> $ hg status
>> M src/jdk.jdwp.agent/share/native/libjdwp/invoker.c
>> R test/com/sun/jdi/OomDebugTest.java
>> ? files.list
>>
>> $ hg diff -r 13877 `cat files.list `
>> <empty output>
>>
>> $ hg diff src/jdk.jdwp.agent/share/native/libjdwp/invoker.c
>> diff -r 96b1cfa80016 src/jdk.jdwp.agent/share/native/libjdwp/invoker.c
>> --- a/src/jdk.jdwp.agent/share/native/libjdwp/invoker.c Sat Apr 02 
>> 05:30:48 2016
>> +0200
>> +++ b/src/jdk.jdwp.agent/share/native/libjdwp/invoker.c Wed Apr 06 
>> 14:57:33 2016
>> -0700
>> @@ -209,47 +209,6 @@ createGlobalRefs(JNIEnv *env, InvokeRequ
>>       }
>>
>>       return error;
>> -}
>> -
>> -/*
>> - * Delete global references from the request which got put there 
>> before a
>> - * invoke request was carried out. See fillInvokeRequest() and 
>> invoker invoke*()
>> - * impls.
>> - */
>> -static void
>> -deleteGlobalRefs(JNIEnv *env, InvokeRequest *request)
>> -{
>> - void *cursor;
>> - jint argIndex = 0;
>> - jvalue *argument = request->arguments;
>> - jbyte argumentTag = firstArgumentTypeTag(request->methodSignature, 
>> &cursor);
>> -
>> - if (request->clazz != NULL) {
>> - tossGlobalRef(env, &(request->clazz));
>> - }
>> - if (request->instance != NULL) {
>> - tossGlobalRef(env, &(request->instance));
>> - }
>> - /* Delete global argument references */
>> - while (argIndex < request->argumentCount) {
>> - if ((argumentTag == JDWP_TAG(OBJECT)) ||
>> - (argumentTag == JDWP_TAG(ARRAY))) {
>> - if (argument->l != NULL) {
>> - tossGlobalRef(env, &(argument->l));
>> - }
>> - }
>> - argument++;
>> - argIndex++;
>> - argumentTag = nextArgumentTypeTag(&cursor);
>> - }
>> - /* Delete potentially saved return values */
>> - if ((request->invokeType == INVOKE_CONSTRUCTOR) ||
>> - (returnTypeTag(request->methodSignature) == JDWP_TAG(OBJECT)) ||
>> - (returnTypeTag(request->methodSignature) == JDWP_TAG(ARRAY))) {
>> - if (request->returnValue.l != NULL) {
>> - tossGlobalRef(env, &(request->returnValue.l));
>> - }
>> - }
>>   }
>>
>>   static jvmtiError
>> @@ -777,13 +736,6 @@ invoker_completeInvokeRequest(jthread th
>>           (void)outStream_writeObjectRef(env, &out, exc);
>>           outStream_sendReply(&out);
>>       }
>> -
>> - /*
>> - * At this time, there's no need to retain global references on
>> - * arguments since the reply is processed. No one will deal with
>> - * this request ID anymore, so we must call deleteGlobalRefs().
>> - */
>> - deleteGlobalRefs(env, request);
>>   }
>>
>>   jboolean
>>
>> $ hg diff -r 13877 src/jdk.jdwp.agent/share/native/libjdwp/invoker.c
>> <empty output>
>>