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

[serguei.spitsyn at oracle.com]

Hi Dan,

The fix looks right.
Thank you for taking care about the hs-rt stability!
It makes sense to back the fix of the 4858370 out at least because
it introduced new test that started failing intermittently.

The following 3 bugs have similar manifestation and related to the 
remote invocation
of a method or a constructor in the target VM:
https://bugs.openjdk.java.net/browse/JDK-8152586
https://bugs.openjdk.java.net/browse/JDK-8152686
https://bugs.openjdk.java.net/browse/JDK-8152985


I'm still in process of isolation of the JDK-8152586 regression.
I took a wrong path at some point and need to redo some of my steps.

Thanks,
Serguei


On 4/6/16 16:34, Daniel D. Daugherty wrote:
> 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>
>