[8u-dev] RFR (S): JDK-8194653: Deadlock involving FileSystems.getDefault and System.loadLibrary call

[David Holmes]

Hi Ryan,

First with regard to the mailing list for the RFR it is not the bug 
filing category that determines this (though there is usually a 
correlation) but the location of the files where you actually made 
changes. In this case, in the current form, as hotspot changes, it 
should have been reviewed on hotspot-runtime-dev. But I don't want this 
to be hotspot changes. :)

On 24/04/2019 12:47 am, Sciampacone, Ryan wrote:
> 
> Alan / David,
> 
> I agree this could have also been a class library only solution.  Looking at the problem I saw a few things:
> 
> - Just above the patched code in create_vm() there is similar code for a JSR 292 deadlock problem where classes are pre-loaded as a solution.  I felt this matched the type of fix I was making here.
> - There is plenty of other class pre loading / initialization done in create_vm()

The VM controls the basic bootstrapping initialization process** to get 
the core of the core libraries initialized in the right order so that 
the VM can execute the initialization code in a sane fashion. JSR-292 
code is tightly connected to the VM, so are String, Class, and a bunch 
of others like exceptions. The FileSystem code is not part of this core 
and the VM doesn't need to know about it. This is a library problem that 
can and should be fixed in the library code.

** With the introduction of the module system a lot of this was pushed 
back into the Java library code itself.

> - If there was a need to put an early switch on the pre-load of this class, I felt this be more natural to wrap that here

Not sure what you mean by "early switch", if you mean a flag to control 
whether or not to do the eager-initialization then a Java property could 
be used for that. Though I'd argue against making this optional unless 
there is a significant penalty for always doing the early init - is there?

Thanks,
David

> - The problem doesn't exist at jdk9+ because the classes causing this problem get loaded and initialized as part of the initialization sequence "naturally" (ie: I see no indication that they are loaded to solve this specific problem).  I agree that jdk8 is unlikely to have dramatic changes in initialization moving forward, but putting it as part of (to me) the more streamlined create_vm() function made sense from a "safety" perspective as it felt more controlled and unlikely to be perturbed by any other change in the initialization code flow (class library or vm).
> 
> To follow on this last couple of points, as noted in the defect (and Brian Burkhalter also observes through running the reproducer test), this doesn't happen at jdk9+ because initialization has changed and the class got loaded "for free".  The reproducer test certainly applies to 13 (it's a valid check) but you can also eyeball the classes loaded and see the effect.
> 
> On 4/22/19, 11:42 PM, "Alan Bateman" <Alan.Bateman at oracle.com> wrote:
> 
>      On 22/04/2019 23:11, David Holmes wrote:
>      > Hi Ryan,
>      >
>      > On 23/04/2019 7:04 am, Sciampacone, Ryan wrote:
>      >>
>      >> Bug: https://bugs.openjdk.java.net/browse/JDK-8194653
>      >> Webrev: http://cr.openjdk.java.net/~phh/8194653/webrev.00/
>      >
>      > Why does this need to be pushed to the VM? Why not do the
>      > pre-loading/initializing at the Java level?
>      Ryan - do you have a test case that demonstrates the issue with the main
>      line (jdk/jdk)? I think we need that in order to study this issue a bit
>      more closely and see what options are available. I see Paul has pasted
>      in a stack trace from January and I think it would be interesting to see
>      what command line options were used. I don't have a good sense yet how
>      this might be fixed but it may need work in FilePermission and/or
>      initializing the default file system provider at a different time during
>      the startup.
>      
>      -Alan
>      
>