On Mon, 4 Dec 2023 12:33:26 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:
> 
>   improve handling of nonexisting files

src/hotspot/os/aix/os_aix.cpp line 203:

> 201: constexpr int max_handletable = 1024;
> 202: static int g_handletable_used = 0;
> 203: static struct handletableentry g_handletable[max_handletable] = 
> {{0,0,0,0}};

style nits:
- we usually write the * behind type, not before var name
- `{{0,0}}` -> insert spaces

src/hotspot/os/aix/os_aix.cpp line 1159:

> 1157:     result = ::dlopen(filename, dflags);
> 1158:     if (result != nullptr) {
> 1159:       assert(false, "dll_load: Could not stat() file %s, but dlopen() 
> worked; Have to improve stat()", filename);

use assert(result != nullptr) and remove condition

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

PR Review Comment: https://git.openjdk.org/jdk/pull/16920#discussion_r1413843503
PR Review Comment: https://git.openjdk.org/jdk/pull/16920#discussion_r1413846111

Reply via email to