On Mon, 18 Dec 2023 11:30:59 GMT, Joachim Kern <jk...@openjdk.org> wrote:
>> On AIX, repeated calls to dlopen referring to the same shared library may >> result in different, unique dl handles to be returned from libc. In that it >> differs from typical libc implementations that cache dl handles. >> >> This causes problems in the JVM with code that assumes equality of handles. >> One such problem is in the JVMTI agent handler. That problem was fixed with >> a local fix to said handler >> ([JDK-8315706](https://bugs.openjdk.org/browse/JDK-8315706)). However, this >> fix causes follow-up problems since it assumes that the file name passed to >> `os::dll_load()` is the file that has been opened. It prevents efficient, >> os_aix.cpp-local workarounds for other AIX issues like the *.so/*.a duality. >> See [JDK-8320005](https://bugs.openjdk.org/browse/JDK-8320005). As such, it >> is a hack that causes other, more uglier hacks to follow (see discussion of >> https://github.com/openjdk/jdk/pull/16604). >> >> We propose a different, cleaner way of handling this: >> >> - Handle this entirely inside the AIX versions of os::dll_load and >> os::dll_unload. >> - Cache dl handles; repeated opening of a library should return the cached >> handle. >> - Increase handle-local ref counter on open, Decrease it on close >> - Make sure calls to os::dll_load are matched to os::dll_unload (See >> [JDK-8320830](https://bugs.openjdk.org/browse/JDK-8320830)). >> >> This way we mimic dl handle equality as it is implemented on other >> platforms, and this works for all callers of os::dll_load. > > Joachim Kern has updated the pull request incrementally with one additional > commit since the last revision: > > Followed Thomas proposals I like getting rid of `#ifdef AIX` in shared code. The change is not simple, but looks basically good to me. I'll take a closer look when I find more time. I have some coding style requests. Please also see https://wiki.openjdk.org/display/HotSpot/StyleGuide (especially section Whitespace). src/hotspot/os/aix/porting_aix.cpp line 964: > 962: > 963: return libpath; > 964: Empty line could get removed. src/hotspot/os/aix/porting_aix.cpp line 985: > 983: if (strchr(path2, '/')) { > 984: stringStream combined; > 985: if (*path2 == '/' || *path2 == '.') We usually use `{` and `}` unless for extremely simple substatements on the same line src/hotspot/share/runtime/os.hpp line 1068: > 1066: static bool pd_dll_unload(void* libhandle, char* ebuf, int ebuflen); > 1067: > 1068: Please remove empty lines. ------------- PR Review: https://git.openjdk.org/jdk/pull/16920#pullrequestreview-1787236876 PR Review Comment: https://git.openjdk.org/jdk/pull/16920#discussion_r1430362306 PR Review Comment: https://git.openjdk.org/jdk/pull/16920#discussion_r1430366154 PR Review Comment: https://git.openjdk.org/jdk/pull/16920#discussion_r1430367434
