RFR: 8282475: SafeFetch should not rely on existence of Thread::current

David Holmes dholmes at openjdk.java.net
Mon Mar 7 12:19:06 UTC 2022


On Mon, 7 Mar 2022 11:29:08 GMT, Johannes Bechberger <duke at openjdk.java.net> wrote:

> The WXMode for the current thread (on MacOS aarch64) is currently stored in the thread class which is unnecessary as the WXMode is bound to the current OS thread, not the current instance of the thread class.
> This pull request moves the storage of the current WXMode into a thread local global variable in `os` and changes all related code. SafeFetch depended on the existence of a thread object only because of the WXMode. This pull request therefore removes the dependency, making SafeFetch usable in more contexts.

Hi Johannes,

The general idea seems good (pity it touches so many files, but then I've never liked any of this WX support precisely because it is so invasive of shared code). I agree that safeFetch should not have become dependent on Thread::current existing, but I have to wonder whether we can just skip the WX code if there is no current thread? If the thread is not attached to the VM then what does it even mean to manipulate the WX state of an unknown thread?

That aside, with this change I think we can move the conditional WX code out of the shared os.hpp and bury it down in os_bsd_aarch64.hpp where it actually belongs. 

I'd even like to see threadWXSetters.inline.hpp moved to being in src/os_cpu/bsd_aarch64/ if feasible - I'm not sure what include would be needed for the callsites to function - os.hpp I presume?

Thanks,
David

src/hotspot/share/runtime/threadWXSetters.inline.hpp line 33:

> 31: #if defined(__APPLE__) && defined(AARCH64)
> 32: 
> 33: #include "runtime/thread.inline.hpp" // dependencies require this include

I can't see how this include is needed now.

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

PR: https://git.openjdk.java.net/jdk/pull/7727


More information about the serviceability-dev mailing list