RFR: 8268364: jmethod clearing should be done during unloading [v2]
Coleen Phillimore
coleen.phillimore at oracle.com
Thu Jul 1 19:37:04 UTC 2021
On 7/1/21 12:38 PM, Erik Österlund wrote:
> On Thu, 1 Jul 2021 15:50:26 GMT, Coleen Phillimore <coleenp at openjdk.org> wrote:
>
>>> This patch moves the jmethod clearing to ClassLoaderData::unload() but also adds a check to Method::checked_resolved_jmethod_id() to handle the case where ZGC may be unloading a class but not have gotten to ClassLoaderData::unload() yet. JVMTI will read a NULL method for checked_resolved_jmethod_id() in this case, and not get a Method that will shortly, or has already been reclaimed in the Metaspace destructor.
>>> Since I was there, I also added Method::is_valid_method() check to checked_resolve_jmethod_id. I don't think it's expensive anymore but it could be added under DEBUG. Either way method->method_holder()->is_loader_alive() will crash if !is_valid_method so we should leave it. As I wrote in the related issues, the bogus Method may have been because of a previous set of bugs with post_compiled_method_load events.
>>>
>>> Tested with tiers 1-6 on linux-x64-debug and 1-3 on windows-x64-debug.
>>>
>>> Also ran vmTestbase/nsk/{jdi,jvmti} tests with VM_OPTIONS=-XX:+UseZGC -XX:ZCollectionInterval=0.01 -XX:ZFragmen
>>> tationLimit=0
>> Coleen Phillimore has updated the pull request incrementally with two additional commits since the last revision:
>>
>> - ooops fixed typo
>> - Add a comment about is_loader_alive.
> I did wonder if this was an issue but thought that if you are racingly using a jmethodid that is uncontrollably being unloaded (i.e. you are not holding the class alive in any way at all), then you are using the APIs in a seemingly buggy way that may or may not work the way you expect it to. Nevertheless, bugging out without a crash does seem better, and it did annoy me that these jmethodids are the only things unlinked that late.
> Thanks for fixing this properly. This is the way I thought about fixing it, when I was not sure if it needed fixing or not. Sounds like it did need fixing after all.
Erik, thank you for confirming my diagnosis of the problem and
solution. Ioi had found this through code inspection and carefully
reading JVMTI together helped. There weren't any test failures, and in
fact, this bug is getting marked with noreg-hard.
Thanks!
Coleen
>
> -------------
>
> Marked as reviewed by eosterlund (Reviewer).
>
> PR: https://git.openjdk.java.net/jdk/pull/4643
More information about the hotspot-dev
mailing list