On Wed, 15 Jan 2025 11:23:29 GMT, Magnus Ihse Bursie <i...@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. >> >> In this case, the `DllMain` function did two things: >> >> 1) At startup, it called SetModuleHandle. This has been moved to the >> `JNI_OnLoad` function, which is called by the JVM right after loading the >> DLL, or in the case of a static build, when the `awt` native library has >> been requested. >> >> 2) At DLL unload -- for debug builds only -- it "disabled" the mutexes for >> the DTrace and DMem debug systems. In this case, "disable" means writing >> NULL to the mutexes, causing any further calls to the continue without >> locking, since the enter/exit calls only do the locking if the mutex is not >> NULL. (This is pre-existing code so I am not discussing the soundness of >> this approach.) >> >> But why did we need to do that? After the DLL unloading, which is done by >> Windows when the process is exiting, no code should be executing in AWT, >> right? No, wrong. There are three static objects, one instance of >> AwtDebugSupport and two instances of GDIHashtable, and the destructors of >> these objects are called by Windows at the time of process shutdown. I have >> not been able to confirm that the `DllMain` code is guaranteed to be called >> before the destructors are called, so I guess the fact that this ever worked >> has just been a lucky coincidence. >> >> I have solved this by disabling the mutexes in the destructors themselves, >> thereby guaranteeing that they are disabled before the last few calls to the >> DTrace/DMem calls are made. There is no guarantee in which order these >> destructors are made, so I do the same on both locations. (There is no bad >> effect from calling these twice in a row, it's just setting a mutex to NULL). >> >> Finally, I have wrapped the `DEF_JNI_OnLoad` function in `extern "C" { ... >> }`. This is needed since DEF_JNI_OnLoad is a macro that actually creates two >> functions, where one is a wrapper calling the other which has a generated >> name, and without this, the C++ name mangling messed thi... > > Magnus Ihse Bursie has updated the pull request with a new target base due to > a merge or a rebase. The pull request now contains five commits: > > - Merge branch 'master' into dll-main-in-libawt > - Simplify patch > - Merge branch 'master' into dll-main-in-libawt > - Merge branch 'master' into dll-main-in-libawt > - 8346388: Cannot use DllMain in libawt for static builds Looks good to me. src/java.desktop/windows/native/libawt/windows/awt_Toolkit.cpp line 108: > 106: * of a dynamic library build, or the .EXE in case of a static build. > 107: */ > 108: HMODULE GetAwtModuleHandle() { Should it be `static`? The function `GetAwtModuleHandle` isn't used outside of this translation unit and can be marked `static` to explicitly hide from other translation units unless it stops working. ------------- Marked as reviewed by aivanov (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/22790#pullrequestreview-2562836866 PR Review Comment: https://git.openjdk.org/jdk/pull/22790#discussion_r1922665836
