RFR: 8346433: Cannot use DllMain in hotspot for static builds [v2]

[David Holmes]

On Mon, 13 Jan 2025 13:50:54 GMT, Magnus Ihse Bursie <ihse at openjdk.org> wrote:

>> To be able to properly support static builds on Windows in [JDK-8346377](https://bugs.openjdk.org/browse/JDK-8346377), we cannot use DllMain, for two reasons:
>> 
>> 1) This is not called for statically linked libraries, and
>> 2) There are multiple DllMain definitions throughout the JDK native libraries, causing name collisions.
>> 
>> While it could have been possible to keep the DllMain function for non-static builds and just use an alternative solution for static builds, I think it is preferable to have a single solution that works as well for both static and dynamic builds.
>> 
>> The DllMain in hotspot is doing work both at DLL load time, and at DLL unload time. Let's go through them to see why this patch is safe.
>> 
>> During DLL load time, the library handle is set, and the hi-res timer is initialized. The `pre_initialize` method from `WindowsDbgHelp` and `SymbolEngine` is called. These two are basically identical; both setup a critical section (a Windows mutex). However, this mutex is only used from a stack local guard object, which is allocated only when calls are made into these classes, which are not happening at bootstrapping time, so this shift in initialization time is harmless.
>> 
>> That shift in time is also very small. The `DllMain` method is called by Windows when the DLL is loaded (by libjli), and the very first thing that JLI does after that is to call `JNI_CreateJavaVM`, which ends up calling  `Threads::create_vm`, which calls `os::init` early on.
>
> Magnus Ihse Bursie has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains five additional commits since the last revision:
> 
>  - Merge branch 'master' into dll-main-in-hotspot
>  - Add explanation
>  - Update comments and copyright year
>  - Restore DllMain
>  - 8346433: Cannot use DllMain in hotspot for static builds

I'm okay with this refactoring.

I have suggested some changes to three comments, noting that the static case is not really "as early as possible" but rather it is done at the first platform specific initialization function called. If you really wanted/needed it "as early as possible" then a `WINDOWS_ONLY(...)` call at the start of `Threads::create_vm` would be okay too - and that would help with the crash reporting problem.

FWIW I tracked down the initialization/termination order information and as you stated in the other PR the atexit handlers do run after dllMain - so no concerns about potentially changing things in that regard.

https://learn.microsoft.com/en-us/cpp/build/run-time-library-behavior?view=msvc-170

Thanks

src/hotspot/os/windows/symbolengine.hpp line 61:

> 59:   void print_state_on(outputStream* st);
> 60: 
> 61:   // Called as early as possible

Suggestion:

// Called  at DLL_PROCESS_ATTACH for dynamic builds, and from os::init() for static builds.

src/hotspot/os/windows/windbghelp.cpp line 144:

> 142: }
> 143: 
> 144: // Called as early as possible

Suggestion:

// Called  at DLL_PROCESS_ATTACH for dynamic builds, and from os::init() for static builds.

src/hotspot/os/windows/windbghelp.hpp line 70:

> 68:   void print_state_on(outputStream* st);
> 69: 
> 70:   // Called as early as possible

Suggestion:

// Called  at DLL_PROCESS_ATTACH for dynamic builds, and from os::init() for static builds.

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

PR Review: https://git.openjdk.org/jdk/pull/22793#pullrequestreview-2548859807
PR Review Comment: https://git.openjdk.org/jdk/pull/22793#discussion_r1914279133
PR Review Comment: https://git.openjdk.org/jdk/pull/22793#discussion_r1914279431
PR Review Comment: https://git.openjdk.org/jdk/pull/22793#discussion_r1914279535