RFR: 8317951: Refactor loading of zip library to help resolve JDK-8315220 [v3]

[Markus Grönlund]

On Fri, 20 Oct 2023 06:20:22 GMT, David Holmes <dholmes at openjdk.org> wrote:

>> Markus Grönlund has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   do not exit vm from HeapCompression usage
>
> src/hotspot/share/utilities/zipLibrary.cpp line 65:
> 
>> 63:   assert(_zip_handle != nullptr, "invariant");
>> 64:   void* func = os::dll_lookup(_zip_handle, name);
>> 65:   if (func == nullptr && vm_exit_on_failure) {
> 
> This seems to be unraveling somewhat. If we don't exit-on-failure then the callers have to be prepared for that failure and it isn't obvious to me that is always the case. And this is no longer just a refactoring. So I think a separate issue would need to be filed to look at this change in behaviour.

I understand your point. The complication comes about from the usage in HeadDumperCompression, which only wants to reuse some functions in the zip library, init_params() and compress(). This caller is only invoked by means of an externally invoked jcmd, HeapDumpDCmd, iff the gzip compression level is set. To have this jcmd exit the VM is a departure from existing behaviour. So this change aligns with the existing behavior.

The other client, which uses this in a more structured way, is ClassLoader:. It is used also by the JNI entry point JVM_LoadZipLibrary(). Even from this JNI entry point, the behaviour is to perform a vm_exit on failure.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/16149#discussion_r1366709026