Unsafe and JPMS

[Jeremy Manson]

On Fri, Jan 19, 2018 at 11:52 AM, Alan Bateman <Alan.Bateman at oracle.com>
wrote:

> On 19/01/2018 18:31, Jeremy Manson wrote:
>
> :
>
> 1) testStaticsDiscovery(com.google.common.profile.HeapInspector
> Test)java.lang.reflect.InaccessibleObjectException: Unable to make field
> final jdk.internal.loader.URLClassPath jdk.internal.loader.ClassLoaders$AppClassLoader.ucp
> accessible: module java.base does not "opens jdk.internal.loader" to
> unnamed module @3e11f9e9; did you mean --add-opens=java.base/jdk.inte
> rnal.loader=ALL-UNNAMED
> at java.base/java.lang.reflect.AccessibleObject.checkCanSetAcce
> ssible(AccessibleObject.java:345)
> at java.base/java.lang.reflect.AccessibleObject.checkCanSetAcce
> ssible(AccessibleObject.java:281)
> at java.base/java.lang.reflect.Field.checkCanSetAccessible(Field.java:176)
> at java.base/java.lang.reflect.Field.setAccessible(Field.java:170)
> at com.google.common.profile.HeapInspector.getClassFields(HeapI
> nspector.java:1234)
>
> The class name hints that this is a HeapInspector walking the object
> graph. I assume it's not deliberately trying to hack the ucp field (btw: is
> the "did you mean ..." line a Google customization?)
>

Correct - it is just walking the transitively reachable fields.  And yes,
we stuck that in there to clarify what you could do to work around.  (We'd
be happy to upstream the patch if folks think it would be desirable - it's
not very big).

The tool may have worked unchanged since JDK 1.4 but it now finds itself in
> a new world where there are modules that are trying to encapsulate their
> internals. In this case the java.base module does not want to open
> jdk.internal.loader (it's new in JDK 9 so this is why it is not open). This
> is the tension that is discussed here many times. All I'm saying is that
> the path for diagnostic tooling like this is the tool agent route. Tool
> agents get encapsulation busting powers at the cost of using tool APIs and
> being deployed as agents.
>


Right.  I guess the question I would have about that is: if I am using an
API that is explicitly called "Unsafe", then aren't I basically saying that
I'm willing to live without the benefits of encapsulation?  In this case,
the tradeoff is that we have to make tools that are significantly harder to
write and deploy.  If I actually have to go down to JVMTI, it's even worse
- I would have to write platform specific code in a different language in
order to accomplish what I want.

And using Unsafe for diagnostic purposes violates none of the stated goals
of JPMS, as far as I can tell.

(I'm complaining, but we've made deploying agents very easy at Google.  I'm
mostly dreading the idea that we would have to rewrite it in a way that
would take significant time or be harder for people to understand, or that
external developers would have to worry about the deployment issues, given
the hoops we had to jump through to make deploying agents easy.)

If Launcher-Agent-Class is for executable JAR files, then that's basically
> a non-starter for library classes, too.
>
> Also, it's worth pointing out that even if it feels wrong to you, it has
> worked in the platform for many years - the cost I'm talking about was
> written for Java 4, and hasn't needed to be changed since then.
>
> I also think that diagnostic code is a red herring here.  The OSS use
> cases I pointed to are not in diagnostic code.
>
> Sure, diagnostics tools are special but at least there are APIs and
> deployment options for these types of tools.
>
> One of the libraries you listed seems to a serialization library. The
> conflict between legacy serialization and encapsulation is irreconcilable.
> While somewhat sad, this means that custom serialization libraries need to
> use the updated ReflectionFactory APIs in conjunction with unsafe. The API
> additions are in JDK 9 and were back-ported to JDK 8u  to make it easier
> for serialization libraries that want to compile to JDK 8. I can't tell if
> the library you listed has been updated to use these APIs or not.
>

The critical thing there is that these things are going to receive
continued support for module-busting behavior.  Which is really what I want
to make sure happens - all of those APIs are in jdk.unsupported. :)

The other place I saw that this came up is the cglib code generation
library, which now uses Unsafe to access ClassLoader.defineClass (for any
ClassLoader).  This is useful for generating classes for dependency
injection / mocking / that kind of thing.  It is a violation of
encapsulation, but it is done with a very specific software engineering
goal in mind.  It was never a terribly good approach, but it a library that
is now depended upon by a very large amount of code in the wild.

(I've thought it might be interesting to replace dynamic frameworks like
these with something based on invokedynamic, but I'm not sure how we would
indicate which method calls we want to replace with indys without changes
to the platform we probably don't want to make.)

Jeremy