> This PR replaces the withdrawn PR #1348. This PR tries to fix the underlying > problem, rather than fix the tests. > > The problem is that a number of JDI tests create objects on the debugger side > with calls to `newInstance()`. However, on the debugee side, these new > instances will only be held on to by a `JNIGlobalWeakRef`, which means they > could be collected at any time, even before `newInstace()` returns. A number > of JDI tests get spurious `ObjectCollectedException` thrown at them, which > results in test failures. To make these objects stick around, a call to > `disableCollection()` is typically needed. > > However, as pointer out by @plummercj in > [JDK-8255987](https://bugs.openjdk.java.net/browse/JDK-8255987): > >> Going back to the spec, ObjectReference.disableCollection() says: >> >> "By default all ObjectReference values returned by JDI may be collected at >> any time the target VM is running" >> >> and >> >> "Note that while the target VM is suspended, no garbage collection will >> occur because all threads are suspended." >> >> But no where does is say what is meant by the VM running or being suspended, >> or how to get it in that state. One might assume that this ties in with >> VirtualMachine.suspend(), but it says: >> >> "Suspends the execution of the application running in this virtual machine. >> All threads currently running will be suspended." >> >> No mention of suspending the VM, but that certainly seems to be what is >> implied by the method name and also by the loose wording in >> disableCollection(). > > Most of our spuriously failing tests do actually make a call to > `VirtualMachine.suspend()`, presumably to prevent objects from being garbage > collected. However, the current implementation of `VirtualMachine.suspend()` > will only suspend all Java threads. That is not enough to prevent objects > from being garbage collected. The GC can basically run at any time, and there > is no relation to whether all Java threads are suspended or not. > > However, as suggested by @plummercj, we could emulate the behaviour implied > by the spec by letting a call to `VirtualMachine.suspend()` also convert all > existing JDI objects references to be backed by a (strong) `JNIGlobalRef` > rather than a (weak) `JNIGlobalWeakRef`. That will not prevent the GC from > running, but it will prevent any object visible to a JDI client from being > garbage collected. Of course, a call to `VirtualMachine.resume()` would > convert all references back to being weak again. > > This patch introduces the needed functions in `libjdwp` to "pin" and "unpin" > all objects. These new functions are then used by the underpinnings of > `VirtualMachine.suspend()` and `VirtualMachine.resume()` to implement the > behaviour described above. > > Note that there are still a few tests that needed adjustments to guard > against `ObjectCollectionException`. These are: > - *vmTestbase/nsk/jdi/ArrayType/newInstance/newinstance004.java* - This test > seems to have been forgotten by > [JDK-8203174](https://bugs.openjdk.java.net/browse/JDK-8203174), which did a > similar fix in the other `ArrayType/newinstance` tests. > - > *vmTestbase/nsk/jdi/VMOutOfMemoryException/VMOutOfMemoryException001/VMOutOfMemoryException001.java* > - We just want to allocate as much as we can, so catching an ignoring > `ObjectCollectedException` seems reasonable here. > - *vmTestbase/nsk/share/jdi/sde/SDEDebuggee.java* - We still want to prevent > `TestClassLoader` from being unloaded to avoid invalidating code locations. > - > *vmTestbase/nsk/jdi/ReferenceType/instances/instances002/instances002.java* - > This test keeps the VM suspended, and then expects objects to be garbage > collected, which they now won't. > > Testing: > - More than 50 iterations of the `vmTestbase/nsk/jdi` and > `vmTestbase/nsk/jdwp` test suites, using various GC, both in mach5 and > locally.
Per Liden has updated the pull request incrementally with one additional commit since the last revision: Fix copyright ------------- Changes: - all: https://git.openjdk.java.net/jdk/pull/1595/files - new: https://git.openjdk.java.net/jdk/pull/1595/files/8fe1e52d..55cd2462 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=1595&range=02 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=1595&range=01-02 Stats: 4 lines in 4 files changed: 0 ins; 0 del; 4 mod Patch: https://git.openjdk.java.net/jdk/pull/1595.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/1595/head:pull/1595 PR: https://git.openjdk.java.net/jdk/pull/1595
