Re: Unsafe and JPMS

classic Classic list List threaded Threaded
6 messages Options
Reply | Threaded
Open this post in threaded view
|

Re: Unsafe and JPMS

Alan Bateman
Moving this thread to jigsaw-dev as that is where this topic has been
discussed ad nauseam.

On 19/01/2018 01:28, Jeremy Manson wrote:

> Hey folks,
>
> I know this has come up before, but I wanted to revive the question.  As we
> play more and more with JPMS and Java 9, we're finding more and more places
> where Unsafe is now the only feasible way to achieve what we want to
> achieve.
>
> The big use case is, of course, access to the internals of modularized
> libraries in ways that would have used standard reflection in Java 8 and
> earlier, but for which we now fail with an exception.
Are they really failing with exceptions? I ask because the standard and
JDK modules open most of their packages in JDK 9 and JDK 10 for
so-called "deep reflection". If there are libraries on the class path
hacking into JDK internals then they have a good chance that they will
continue to work, assuming the internals haven't changed. There may be a
warning of course but that is to help identify code that may break in
the future.

>
> The answer of adding command line flags doesn't really work for general
> purpose libraries.  If I want to do this in (for example) Guava, asking
> every Guava user to add add-opens on the command line is a complete
> non-starter.  Many people have no idea what libraries they are using.
>
> The answer of creating an agent is also a non-starter for similar reasons.
>
> The answer of using privateLookupIn assumes that you have access to or
> control over the target class, which is often not the case, especially if
> you are introspecting into JDK internals.
>
> As people within Google try Java 9, this is coming up in quite a bit of our
> infrastructure, especially in diagnostic code.  The latest place I've found
> it was a tool that printed out / counted the number of objects reachable
> from a root object; it died when it got into a JDK library.
JVM TI is the API for walking the heap of a running VM. That should work
as before.

Diagnostic tools doing heap walking in Java feels like something for a
java agent rather than a general purpose library on the class path
(ignoring serialization for now as that's a discussion in itself). If
the tooling is using reflection to potentially access every field of
every object on the heap then it may need changes (maybe now if it finds
itself walking references to objects of classes in packages that are new
in JDK 9). The Instrumentation API has the power to do that of course as
it can open any package to anyone. This means it can get full-power
Lookup to any class in the JDK with privateLookupIn. Agents need to
guard this capability closely of course.

As regards starting java agents then one addition in JDK 9 to look at is
the Launcher-Agent-Class attribute that executable JAR files including
an agent can use.

> :
>
> So... OpenJDK has this commitment not to replace Unsafe without providing
> supported replacements for its functionality and a transition period.  Does
> that include the functionality that Unsafe can break module encapsulation?
I'm not aware of any current proposals to remove Unsafe. I'm sure
degrading Unsafe will come up once Panama and maybe Valhalla are further
along.

For now, I think the right thing is to continue the effort to identify
candidate APIs (where it makes sense) so that code hacking into the JDK
today can migrate in the future. This was the motivation for many new
APIs in JDK 9 and I've no doubt there will be more before the JDK
modules are fully encapsulated.

-Alan.
Reply | Threaded
Open this post in threaded view
|

Re: Unsafe and JPMS

Jeremy Manson
On Fri, Jan 19, 2018 at 5:05 AM, Alan Bateman <[hidden email]>
wrote:

> Moving this thread to jigsaw-dev as that is where this topic has been
> discussed ad nauseam.


Sorry to beat this to death.  The engineering time we're spending on the
Java 9 transition is very costly.  I have a responsibility to make sure I'm
doing what I can to mitigate future costs without concomitant benefits.

On 19/01/2018 01:28, Jeremy Manson wrote:

>
>> Hey folks,
>>
>> I know this has come up before, but I wanted to revive the question.  As
>> we
>> play more and more with JPMS and Java 9, we're finding more and more
>> places
>> where Unsafe is now the only feasible way to achieve what we want to
>> achieve.
>>
>> The big use case is, of course, access to the internals of modularized
>> libraries in ways that would have used standard reflection in Java 8 and
>> earlier, but for which we now fail with an exception.
>>
> Are they really failing with exceptions? I ask because the standard and
> JDK modules open most of their packages in JDK 9 and JDK 10 for so-called
> "deep reflection". If there are libraries on the class path hacking into
> JDK internals then they have a good chance that they will continue to work,
> assuming the internals haven't changed. There may be a warning of course
> but that is to help identify code that may break in the future.



1)
testStaticsDiscovery(com.google.common.profile.HeapInspectorTest)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.internal.loader=ALL-UNNAMED
at
java.base/java.lang.reflect.AccessibleObject.checkCanSetAccessible(AccessibleObject.java:345)
at
java.base/java.lang.reflect.AccessibleObject.checkCanSetAccessible(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(HeapInspector.java:1234)


> The answer of adding command line flags doesn't really work for general
>> purpose libraries.  If I want to do this in (for example) Guava, asking
>> every Guava user to add add-opens on the command line is a complete
>> non-starter.  Many people have no idea what libraries they are using.
>>
>> The answer of creating an agent is also a non-starter for similar reasons.
>>
>> The answer of using privateLookupIn assumes that you have access to or
>> control over the target class, which is often not the case, especially if
>> you are introspecting into JDK internals.
>>
>> As people within Google try Java 9, this is coming up in quite a bit of
>> our
>> infrastructure, especially in diagnostic code.  The latest place I've
>> found
>> it was a tool that printed out / counted the number of objects reachable
>> from a root object; it died when it got into a JDK library.
>>
> JVM TI is the API for walking the heap of a running VM. That should work
> as before.
>
> Diagnostic tools doing heap walking in Java feels like something for a
> java agent rather than a general purpose library on the class path
> (ignoring serialization for now as that's a discussion in itself). If the
> tooling is using reflection to potentially access every field of every
> object on the heap then it may need changes (maybe now if it finds itself
> walking references to objects of classes in packages that are new in JDK
> 9). The Instrumentation API has the power to do that of course as it can
> open any package to anyone. This means it can get full-power Lookup to any
> class in the JDK with privateLookupIn. Agents need to guard this capability
> closely of course.

As regards starting java agents then one addition in JDK 9 to look at is
> the Launcher-Agent-Class attribute that executable JAR files including an
> agent can use.
>

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.

So... OpenJDK has this commitment not to replace Unsafe without providing
>> supported replacements for its functionality and a transition period.
>> Does
>> that include the functionality that Unsafe can break module encapsulation?
>>
> I'm not aware of any current proposals to remove Unsafe. I'm sure
> degrading Unsafe will come up once Panama and maybe Valhalla are further
> along.
>

This was a goal until JEP 260 came along.  My read on this was that it was
simply a matter of time before it came up again.

For now, I think the right thing is to continue the effort to identify
> candidate APIs (where it makes sense) so that code hacking into the JDK
> today can migrate in the future. This was the motivation for many new APIs
> in JDK 9 and I've no doubt there will be more before the JDK modules are
> fully encapsulated.


I generally agree with the principle of making real APIs to do what we need
to do with Unsafe and migrating.  My concern is not the removal of Unsafe,
it's that the replacement won't be equivalent if it doesn't let you cross
module boundaries.

Jeremy
Reply | Threaded
Open this post in threaded view
|

Re: Unsafe and JPMS

Alan Bateman
On 19/01/2018 18:31, Jeremy Manson wrote:

> :
>
> 1)
> testStaticsDiscovery(com.google.common.profile.HeapInspectorTest)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.internal.loader=ALL-UNNAMED
> at
> java.base/java.lang.reflect.AccessibleObject.checkCanSetAccessible(AccessibleObject.java:345)
> at
> java.base/java.lang.reflect.AccessibleObject.checkCanSetAccessible(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(HeapInspector.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?)

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.

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

-Alan

Reply | Threaded
Open this post in threaded view
|

Re: Unsafe and JPMS

Jochen Theodorou
In reply to this post by Jeremy Manson
On 19.01.2018 19:31, Jeremy Manson wrote:
[...]
> I generally agree with the principle of making real APIs to do what we need
> to do with Unsafe and migrating.  My concern is not the removal of Unsafe,
> it's that the replacement won't be equivalent if it doesn't let you cross
> module boundaries.

It won't be equivalent. It can't be or the module system enforced
encapsulation, the big deal of the jpms, would be rendered useless. But
maybe the alternatives are good enough for you. You will still have to
identify the problems points and see if you can replacement with
something else.

bye Jochen

Reply | Threaded
Open this post in threaded view
|

Re: Unsafe and JPMS

Jeremy Manson
In reply to this post by Alan Bateman
On Fri, Jan 19, 2018 at 11:52 AM, Alan Bateman <[hidden email]>
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
Reply | Threaded
Open this post in threaded view
|

Re: Unsafe and JPMS

Alan Bateman
On 20/01/2018 00:07, Jeremy Manson wrote:

> :
>
> 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.
>
DI and other cases with frameworks doing code injection should be
looking at Lookup.defineClass. Frameworks can use this API to inject a
class into an existing runtime package once they have a Lookup object
with the appropriate access. it's not all use-cases but it's the only
supported API in the platform for doing this kind of thing.

-Alan