On 5/11/19 6:39 AM, David Holmes wrote:
On 11/05/2019 2:14 pm, Jean Christophe Beyler wrote:
Isn't that the point? The list returned could have unloaded classes
and we can catch it via this exception (from the comment above
the ReferenceType interface):
* Any method on <code>ReferenceType</code> or which directly or
indirectly takes
* <code>ReferenceType</code> as parameter may throw
* {@link ObjectCollectedException} if the mirrored type has been
unloaded.
Turns out that even in the definedClasses, we can get that exception
so we should check for it while walking the reference types as well?
I understand that the list returned to the "debugger" process may
contain ReferenceTypes for types that have actually been unloaded by
the time it queries them (unless the debuggee is suspended of course).
But I don't see how we can encounter those types while compiling the
list in the debuggee in the first place.
Something seems amiss here ... possibly just my understanding ...
JC quoted the right bits of spec:
* Any method on <code>ReferenceType</code> or which directly or
indirectly takes
* <code>ReferenceType</code> as parameter may throw
* {@link ObjectCollectedException} if the mirrored type has been
unloaded.
The JDI client is working with mirrored type references. The debuggee
has the real type references. If the JDI client has not suspended the
debuggee, then the JDI client must guard against ObjectCollectedException.
At one point, we talked about keeping a reference (global JNI handle)
in the debuggee agent, but we quickly came to the conclusion that we
would never know when it would be safe to release those references.
Dan
David
Jc
*From: *Chris Plummer <[email protected]
<mailto:[email protected]>>
*Date: *Fri, May 10, 2019 at 9:09 PM
*To: *David Holmes, Daniil Titov, OpenJDK Serviceability
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
>>>
>>>
>
>
--
Thanks,
Jc
