Uploaded image for project: 'JDK'
  1. JDK
  2. JDK-8153711

[REDO] JDWP: Memory Leak: GlobalRefs never deleted when processing invokeMethod command

    Details

    • Subcomponent:
    • Resolved In Build:
      b139
    • CPU:
      generic
    • OS:
      generic

      Backports

        Description

        The fix of:
          JDK-4858370 JDWP: Memory Leak: GlobalRefs never deleted when processing invokeMethod command

        was backed out with:
          JDK-8153673 [BACKOUT] JDWP: Memory Leak: GlobalRefs never deleted when processing invokeMethod command

        because of the following regressions:
          JDK-8152686 com/sun/jdi/OomDebugTest.java fails with java.lang.reflect.InvocationTargetException
          JDK-8152586 com/sun/jdi/InterfaceMethodsTest.java fails in hs nightly
          JDK-8152985 nsk/jdi tests fail with unexpected com.sun.jdi.InvocationException

        A corrected fix for the original problem with GlobalRefs memory leak is needed.
        1. JDK-8153711-jdk9-jdk.export.patch
          21 kB
          Severin Gehwolf
        2. JDK-8153711-jdk9-jdk.export.patch
          14 kB
          Severin Gehwolf
        3. JDK-8153711-jdk9-jdk.export.patch
          14 kB
          Severin Gehwolf

          Issue Links

            Activity

            Hide
            sgehwolf Severin Gehwolf added a comment -
            Latest webrev which has the NPE problem on solaris x86_64 fixed. Turns out, getting OomDebugTest working reliably on solaris also fixes the InvokeTest problem. I need to do some more testing, but it's looking good so far:
            http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8153711/webrev.03/
            Show
            sgehwolf Severin Gehwolf added a comment - Latest webrev which has the NPE problem on solaris x86_64 fixed. Turns out, getting OomDebugTest working reliably on solaris also fixes the InvokeTest problem. I need to do some more testing, but it's looking good so far: http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8153711/webrev.03/
            Hide
            sgehwolf Severin Gehwolf added a comment - - edited
            The NPE problem in InvokeTest and the triggered assertion in OomDebugTest is caused by clearing the request's instance and clazz members *after* the IO operation. Like every other failures I've seen for this bug it's not reliably reproducible. Especially not when debugging this itself. It maybe related to the sequence of monitor_exit->perform IO->monitor_enter->toss references. Perhaps there is a schedule where the response has been sent back, the next invoke starts for the same app thread and it is just far enough along so that the tossing of the references becomes observable by the next request. I really don't know.

            However, testing showed that moving this up prevents this assertion from being triggered. Thus, I'm now clearing global references - the ones we can clear before sending back the response to the client - in the same block while still holding the invoker and event handler locks as the rest of the operations being done in completeInvokeRequest. Once the response has been sent to the client there are still two global references to clear: The one for the return value and a possible exception which might have occurred. Since they are required for sending the response we do this after it's been sent.
            Show
            sgehwolf Severin Gehwolf added a comment - - edited The NPE problem in InvokeTest and the triggered assertion in OomDebugTest is caused by clearing the request's instance and clazz members *after* the IO operation. Like every other failures I've seen for this bug it's not reliably reproducible. Especially not when debugging this itself. It maybe related to the sequence of monitor_exit->perform IO->monitor_enter->toss references. Perhaps there is a schedule where the response has been sent back, the next invoke starts for the same app thread and it is just far enough along so that the tossing of the references becomes observable by the next request. I really don't know. However, testing showed that moving this up prevents this assertion from being triggered. Thus, I'm now clearing global references - the ones we can clear before sending back the response to the client - in the same block while still holding the invoker and event handler locks as the rest of the operations being done in completeInvokeRequest. Once the response has been sent to the client there are still two global references to clear: The one for the return value and a possible exception which might have occurred. Since they are required for sending the response we do this after it's been sent.
            Show
            sgehwolf Severin Gehwolf added a comment - Posted for review: http://mail.openjdk.java.net/pipermail/serviceability-dev/2016-September/020407.html
            Hide
            hgupdate HG Updates added a comment -
            URL: http://hg.openjdk.java.net/jdk9/hs/jdk/rev/4c843eb35b8a
            User: dsamersoff
            Date: 2016-09-15 11:09:47 +0000
            Show
            hgupdate HG Updates added a comment - URL: http://hg.openjdk.java.net/jdk9/hs/jdk/rev/4c843eb35b8a User: dsamersoff Date: 2016-09-15 11:09:47 +0000
            Hide
            hgupdate HG Updates added a comment -
            URL: http://hg.openjdk.java.net/jdk9/jdk9/jdk/rev/4c843eb35b8a
            User: lana
            Date: 2016-10-05 20:01:48 +0000
            Show
            hgupdate HG Updates added a comment - URL: http://hg.openjdk.java.net/jdk9/jdk9/jdk/rev/4c843eb35b8a User: lana Date: 2016-10-05 20:01:48 +0000

              People

              • Assignee:
                sgehwolf Severin Gehwolf
                Reporter:
                sspitsyn Serguei Spitsyn
              • Votes:
                0 Vote for this issue
                Watchers:
                6 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved: