On 5/10/19 9:03 PM, Chris Plummer wrote:
On 5/10/19 8:59 PM, David Holmes wrote:
Hi Daniil,
On 11/05/2019 12:10 pm, Daniil Titov wrote:
Please review the change that fixes an intermittent failure of the
test.
The tests checks the implementation of the
com.sun.tools.jdi.ClassLoaderReference class. The problem here is
that while
com.sun.tools.jdi.ClassLoaderReferenceImpl.definedClasses() iterates
over all loaded classes to retrieve a classloader and compares it to
the current one, some of the classes might become unloaded and
garbage collected (e.g.
org.graalvm.compiler.nodes.InliningLog$$Lambda$41.899832640 or
jdk.internal.reflect.GeneratedConstructorAccessor1, etc.). If this
happens then the attempt to retrieve a classloader for the collected
class results in com.sun.jdi.ObjectCollectedException being thrown.
That seems odd to me. If you have a reference to the Class then it
can't be unloaded. I would not expect allClasses() to have
weak-references, so a class should not be unloadable while you are
examining it. Unless it is finding VM anonymous classes (which it
should not!).
I was just typing up something similar. Shouldn't the test do a
vm.suspend() and then call disableCollection() on each class returned
by vm.allClasses()?
Oh wait, this isn't a test. It's part of JDI. I guess it would be up to
the user of ClassLoaderReference.definedClasses() to do the suspend. In
fact I'm not sure there's much purpose in calling
ClassLoaderReference.definedClasses() without suspending first. Even
with your changes, the list returned can end up with references to
unloaded classes.
Chris
Chris
David
-----
The fix catches this com.sun.jdi.ObjectCollectedException and
continues iterating over the rest of the classes.
Webrev: http://cr.openjdk.java.net/~dtitov/8222422/webrev.01
Bug: https://bugs.openjdk.java.net/browse/JDK-8222422
Thanks!
--Daniil
