|
I think there are better ways of
finding out about unloaded classes then getting a list of all the
classes, and then querying each one to see if has been unloaded.
I would argue that if definedClasses()
ever gets ObjectCollectedException, there's probably a bug in the
caller for not doing a vm.suspend(). However, since
definedClasses() does not require the app to be suspended, it is
necessary to defend against ObjectCollectedException as Daniil has
done.
Chris
On 5/10/19 9: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?
Jc
From: Chris
Plummer <[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
>>>
>>>
>
>
--
|
