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

[Markus Grönlund]

On Fri, 20 Oct 2023 09:16:34 GMT, Markus Grönlund <mgronlun at openjdk.org> wrote:

>> 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.

My current take is to keep the existing behaviours of these two callers in order to get rid of the Zip_lock.

Should there be more callers in the future, additional adjustments and refactorings could be performed at that time.

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

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