RFR: JDK-8320890: [AIX] Find a better way to mimic dl handle equality [v6]

[Martin Doerr]

On Mon, 18 Dec 2023 11:30:59 GMT, Joachim Kern <jkern at 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