RFR: 8320302: compiler/arguments/TestC1Globals.java hits SIGSEGV in ContinuationEntry::set_enter_code

David Holmes dholmes at openjdk.org
Thu Feb 1 08:09:03 UTC 2024


On Tue, 16 Jan 2024 23:33:58 GMT, Patricio Chilano Mateo <pchilanomate at openjdk.org> wrote:

> When creating Continuation.enterSpecial/doYield special native nmethods we currently assume the call to nmethod::new_native_nmethod() always succeeds. If the CodeCache happens to be full though, creating the nmethod will fail and we'll hit a SIGSEGV when trying to dereference the return value. 
> 
> Since this happens the first time those methods are resolved, throwing an exception at that point implying that we cannot run virtual threads looks odd from a user perspective. So instead I added a step to initialize the Continuation class at startup so that failure to create those nmethods is treated early as a fatal error as we do with any other critical resource needed by the VM. I measured the added step to the startup sequence to be ~90us, or about 0.3% of the total startup time.
> 
> I verified the fix by running the test mentioned in the bug plus I also run tiers1-5 in mach5.
> 
> Thanks,
> Patricio

Seems reasonable - though I dislike the `fatal` calls.

Will think on it.

Thanks.

src/hotspot/cpu/aarch64/sharedRuntime_aarch64.cpp line 1394:

> 1392:                                               oop_maps,
> 1393:                                               exception_offset);
> 1394:     if (nm == nullptr) fatal("Initial size of CodeCache is too small");

It is a pity we can't just use a return of `nullptr` up the call chain to then have `create_vm` call `vm_exit_during_initialization` rather than using a `fatal` here.

I suppose we could use `vm_exit_during_initialization` here, but it is not obvious this code path can only be executed during the initialization process.

src/hotspot/share/runtime/threads.cpp line 416:

> 414: 
> 415:   initialize_class(vmSymbols::jdk_internal_vm_Continuation(), CHECK);
> 416: }

I don't think this needs to be part of the `Threads` API - just inline it where needed.

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

PR Review: https://git.openjdk.org/jdk/pull/17455#pullrequestreview-1855769037
PR Review Comment: https://git.openjdk.org/jdk/pull/17455#discussion_r1473953007
PR Review Comment: https://git.openjdk.org/jdk/pull/17455#discussion_r1473954439


More information about the hotspot-dev mailing list