Review Request JDK-8240975: Extend NativeLibraries to support explicit unloading
Mandy Chung
mandy.chung at oracle.com
Tue Mar 17 23:09:36 UTC 2020
Hi Alan,
Thanks for the comment. See my comments inlined below.
Here is the updated webrev:
http://cr.openjdk.java.net/~mchung/jdk15/webrevs/8240975/webrev.01
On 3/16/20 3:47 AM, Alan Bateman wrote:
> The difference between the 2 constructors might not be obvious at the
> use sites. I'm just wondering if would be better to use static factory
> methods instead, e.g. the 2-arg constructor could be replaced with a
> trusted(caller, searchLibPath) method that would make it a lot more
> obvious in the places where that will eventually be used.
>
I have similar comment to myself and didn't come up good static factory
method names. I give it a try again: what about newNativeLibraries and
newNativeLibrariesWithNoAutoUnload?
> A small inconsistency is that VM.isSystemDomainLoader is used in
> constructor whereas the other checks for null and platform class
> loader (plus app class loader).
>
Good catch. Revised.
> The Main test could use Path.of("classes"). In setup, dir could be a
> Path and also Path p = Files.createDirectories(...) would allow the
> Files.move to be a bit more readable.
Adjusted.
> I can't quite tell why the test is skipped with -Xcomp but maybe it's
> just too slow and times out?
>
Removed. Cut-n-paste error from an existing test.
> A small suggestion for NativeLibrariesTest is that
> loadWithCustomLoader might be a better name to load p.Test with a
> custom loader. Also noticed libnativeLibrariesTest.c has a 2017 date
> on it.
Fixed.
thanks
Mandy
>
> -Alan.
More information about the core-libs-dev
mailing list