On Thu, 3 Dec 2020 12:55:04 GMT, Per Liden <pli...@openjdk.org> wrote:
> 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. This pull request has now been integrated. Changeset: 79f1dfb8 Author: Per Liden <pli...@openjdk.org> URL: https://git.openjdk.java.net/jdk/commit/79f1dfb8 Stats: 168 lines in 8 files changed: 135 ins; 0 del; 33 mod 8255987: JDI tests fail with com.sun.jdi.ObjectCollectedException Reviewed-by: dholmes, cjplummer ------------- PR: https://git.openjdk.java.net/jdk/pull/1595
