[prev in list] [next in list] [prev in thread] [next in thread]
List: openjdk-serviceability-dev
Subject: Integrated: 8313816: Accessing jmethodID might lead to spurious crashes
From: Jaroslav Bachorik <jbachorik () openjdk ! org>
Date: 2023-11-29 17:33:20
Message-ID: l-EPiXArPC-hnrhOgtEQxm_-GTNIlqWlC4M8OH92kfY=.791190ad-cd1b-42ca-935a-aaea850fea6c () github ! com
[Download RAW message or body]
On Tue, 14 Nov 2023 17:56:09 GMT, Jaroslav Bachorik <jbachorik@openjdk.org> wrote:
> Please, review this fix for a corner case handling of `jmethodID` values.
>
> The issue is related to the interplay between `jmethodID` values and method \
> redefinitions. Each `jmethodID` value is effectively a pointer to a `Method` \
> instance. Once that method gets redefined, the `jmethodID` is updated to point to \
> the last `Method` version. Unless the method is still on stack/running, in which \
> case the original `jmethodID` will be redirected to the latest `Method` version and \
> at the same time the 'previous' `Method` version will receive a new `jmethodID` \
> pointing to that previous version.
> If we happen to capture stacktrace via `GetStackTrace` or `GetAllStackTraces` JVMTI \
> calls while this previous `Method` version is still on stack we will have the \
> corresponding frame identified by a `jmethodID` pointing to that version. However, \
> sooner or later the 'previous' class version becomes eligible for cleanup at what \
> time all contained `Method` instances. The cleanup process will not perform the \
> `jmethodID` pointer maintenance and we will end up with pointers to deallocated \
> memory. This is caused by the fact that the `jmethodID` lifecycle is bound to \
> `ClassLoaderData` instance and all relevant `jmethodID`s will get batch-updated \
> when the class loader is being released and all its classes are getting unloaded.
> This means that we need to make sure that if a `Method` instance is being \
> deallocate the associated `jmethodID` (if any) must not point to the deallocated \
> instance once we are finished. Unfortunately, we can not just update the \
> `jmethodID` values in bulk when purging an old class version - the per \
> `InstanceKlass` jmethodID cache is present only for the main class version and \
> contains `jmethodID` values for both the old and current method versions.
> ~Therefore we need to perform `jmethodID` lookup when we are about to deallocate a \
> `Method` instance and clean up the pointer only if that `jmethodID` is pointing to \
> the `Method` instance which is being deallocated.~
> Therefore, we need to perform `jmethodID` lookup for each method in an old class \
> version that is getting purged, and null out the pointer of that `jmethodID` to \
> break the link from `jmethodID` to the method instance that is about to get \
> deallocated.
> _(For anyone interested, a much lengthier writeup is available in [my \
> blog](https://jbachorik.github.io/posts/mysterious-jmethodid))_
This pull request has now been integrated.
Changeset: cdd1a6e8
Author: Jaroslav Bachorik <jbachorik@openjdk.org>
URL: https://git.openjdk.org/jdk/commit/cdd1a6e851bcaf4a25d4a405b8ee0b0d5b83a4a9
Stats: 206 lines in 8 files changed: 206 ins; 0 del; 0 mod
8313816: Accessing jmethodID might lead to spurious crashes
Reviewed-by: coleenp
-------------
PR: https://git.openjdk.org/jdk/pull/16662
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic