RFR: 8305252: make_method_handle_intrinsic may call java code under a lock

[David Holmes]

On Mon, 3 Apr 2023 19:33:27 GMT, Coleen Phillimore <coleenp at openjdk.org> wrote:

>  Also the method has to call nmethod->flush which is assumed to be something only GC calls.

I'm not familiar with this code but this raises some concerns for me. Is there a way we can test this deallocation path?

Other comments below.

Thanks.

src/hotspot/share/oops/method.cpp line 138:

> 136:   MetadataFactory::free_metadata(loader_data, method_counters());
> 137:   clear_method_counters();
> 138:   // The nmethod will be gone when we get here, for redefinition but not

Nit: the comma should be after redefinition. But is that the only reason to get here? What about regular class unloading?

The method comment on lines 129/130 also needs updating.

src/hotspot/share/oops/method.cpp line 141:

> 139:   // for method handle intrinsics.
> 140:   if (code() != nullptr) {
> 141:     ((nmethod*)_code)->flush();

`flush` acquires the CodeCache_lock - could that be an issue in this call path?

-------------

PR Review: https://git.openjdk.org/jdk/pull/13308#pullrequestreview-1391344080
PR Review Comment: https://git.openjdk.org/jdk/pull/13308#discussion_r1170866278
PR Review Comment: https://git.openjdk.org/jdk/pull/13308#discussion_r1170867580