Re: RFR: JDK-8242888: Convert dynamic proxy to hidden classes

2022-04-19 Thread liach
On Sun, 17 Apr 2022 16:17:30 GMT, liach  wrote:

> Convert dynamic proxies to hidden classes. Modifies the serialization of 
> proxies (requires change in "Java Object Serialization Specification"). Makes 
> the proxies hidden in stack traces. Removes duplicate logic in proxy building.
> 
> The main compatibility changes and their rationales are:
> 1. Modification to the serialization specification: In the "An instance of 
> the class is allocated... The contents restored appropriately" section, I 
> propose explicitly state that handling of proxies are unspecified as to allow 
> implementation freedom, though I've seen deliberate attempts for proxies to 
> implement interfaces with `readResolve` in order to control their 
> serialization behavior.
>- This is for the existing generated constructor accessor is 
> bytecode-based, which cannot refer to hidden classes.
>- An alternative is to preserve the behavior, where the serialization 
> constructor calls `invokespecial` on the closest serializable superclass' 
> no-arg constructor, like in #1830 by @DasBrain.
>- My rationale against preservation is such super calls are unsafe and 
> should be discouraged in the long term. Calling the existing constructor with 
> a dummy argument, as in my implementation, would be more safe.
> 2. The disappearance of proxies in stack traces.
>- Same behavior exists in lambda expressions: For instance, in 
> `((Runnable) () -> { throw new Error(); }).run();`, the `run` method, 
> implemented by the lambda, will not appear in the stack trace, and isn't too 
> problematic.
> 
> A summary of the rest of the changes:
> 1. Merged the two passes of determining module and package of the proxy into 
> one. This reduced a lot of code and allowed anchor class (for hidden class 
> creation) selection be done together as well.
> 2. Exposed internal API for obtaining a full-privileged lookup to the rest of 
> `java.base`. This API is intended for implementation of legacy (pre 
> `MethodHandles.Lookup`) caller sensitive public APIs so they don't need more 
> complex tricks to obtain proper permissions as lookups.
> 3. Implements [8229959](https://bugs.openjdk.java.net/browse/JDK-8229959): 
> passes methods computed by proxy generator as class data to the hidden proxy 
> class to reduce generated proxy class size and improve performance.
> 
> In addition, since this change is somewhat large, should we keep the old 
> proxy generator as well and have it toggled through a command-line flag (like 
> the old v49 proxy generator or the old reflection implementation)?
> 
> Please feel free to comment or review. This change definitely requires a CSR, 
> but I have yet to determine what specifications should be changed.

Fixing deserialization is not particularly hard. A longer term goal is to 
declare a readResolve for proxies (presumably by extra methods on invocation 
handlers) to convert serialization result to something else

-

PR: https://git.openjdk.java.net/jdk/pull/8278


Re: RFR: JDK-8242888: Convert dynamic proxy to hidden classes

2022-04-19 Thread Johannes Kuhn
On Sun, 17 Apr 2022 16:17:30 GMT, liach  wrote:

> Convert dynamic proxies to hidden classes. Modifies the serialization of 
> proxies (requires change in "Java Object Serialization Specification"). Makes 
> the proxies hidden in stack traces. Removes duplicate logic in proxy building.
> 
> The main compatibility changes and their rationales are:
> 1. Modification to the serialization specification: In the "An instance of 
> the class is allocated... The contents restored appropriately" section, I 
> propose explicitly state that handling of proxies are unspecified as to allow 
> implementation freedom, though I've seen deliberate attempts for proxies to 
> implement interfaces with `readResolve` in order to control their 
> serialization behavior.
>- This is for the existing generated constructor accessor is 
> bytecode-based, which cannot refer to hidden classes.
>- An alternative is to preserve the behavior, where the serialization 
> constructor calls `invokespecial` on the closest serializable superclass' 
> no-arg constructor, like in #1830 by @DasBrain.
>- My rationale against preservation is such super calls are unsafe and 
> should be discouraged in the long term. Calling the existing constructor with 
> a dummy argument, as in my implementation, would be more safe.
> 2. The disappearance of proxies in stack traces.
>- Same behavior exists in lambda expressions: For instance, in 
> `((Runnable) () -> { throw new Error(); }).run();`, the `run` method, 
> implemented by the lambda, will not appear in the stack trace, and isn't too 
> problematic.
> 
> A summary of the rest of the changes:
> 1. Merged the two passes of determining module and package of the proxy into 
> one. This reduced a lot of code and allowed anchor class (for hidden class 
> creation) selection be done together as well.
> 2. Exposed internal API for obtaining a full-privileged lookup to the rest of 
> `java.base`. This API is intended for implementation of legacy (pre 
> `MethodHandles.Lookup`) caller sensitive public APIs so they don't need more 
> complex tricks to obtain proper permissions as lookups.
> 3. Implements [8229959](https://bugs.openjdk.java.net/browse/JDK-8229959): 
> passes methods computed by proxy generator as class data to the hidden proxy 
> class to reduce generated proxy class size and improve performance.
> 
> In addition, since this change is somewhat large, should we keep the old 
> proxy generator as well and have it toggled through a command-line flag (like 
> the old v49 proxy generator or the old reflection implementation)?
> 
> Please feel free to comment or review. This change definitely requires a CSR, 
> but I have yet to determine what specifications should be changed.

About deserializing Proxies, this is currently done in 3 steps:

1. Finding the class
2. Allocating a new instance and invoke the constructor of the **first 
non-serializable superclass**
3. Setting the fields of the instance

Only step 2 is problematic here:

1. For a proxy, the class is serialized using a TC_PROXYCLASSDESC. when 
deserialized it invokes `Proxy.getProxyClass` 
(https://github.com/openjdk/jdk/blob/13fb1eed52f1a9152242119969a9d4a0c0627513/src/java.base/share/classes/java/io/ObjectInputStream.java#L891).
  
For this step, it doesn't matter if `Proxy.getProxyClass` returns a normal 
class or a hidden class.
2. Allocating and calling the constructor:  
 This part is currently implemented by spinning a class. The generated 
bytecode looks more or less like this:

anew ProxyClass
invokespecial Object.()

The big lie here is that methods and constructors are different - but 
constructors are just methods, and the verifier makes sure that a constructor 
is called only once, exactly once, and that uninitialized instances don't 
escape.

This doesn't work for hidden classes - as the hidden class can not be named 
in an other class.  
But MethodHandles can do this. Here is one of my old prototypes, based on a 
prototype for implementing reflection using MethodHandles from Mandy Chung: 
https://github.com/dasbrain/jdk/compare/ae45c5de7fc635df4dc86b764405158c245d2765...fbb0a63436f696a85e7039c0e109c379dfa1edce

3. Setting the fields uses deep reflection.   
But the hidden class themself doesn't have any fields - just it's 
superclass `java.lang.reflect.Proxy.h`.  
This is not a problem if the class can be instantiated at all (see step 2).

-

PR: https://git.openjdk.java.net/jdk/pull/8278


Re: RFR: JDK-8242888: Convert dynamic proxy to hidden classes

2022-04-19 Thread Remi Forax
- Original Message -
> From: "liach" 
> To: "core-libs-dev" , "security-dev" 
> 
> Sent: Tuesday, April 19, 2022 3:31:24 AM
> Subject: Re: RFR: JDK-8242888: Convert dynamic proxy to hidden classes

> On Mon, 18 Apr 2022 20:42:48 GMT, Remi Forax  wrote:
> 
>> The third parameter of defineProxy() is a lambda which is called for each 
>> method
>> that can be overriden, either toString/equals/hashCode but also any default
>> methods,
> if the lambda return true, the method is overriden, otherwise the default
> implementation is used.
> 
> Not quite, I mean calling default implementations in the super interface,
> consider:
> 
> interface Root { void cleanUp(); }
> interface FeatureOne extends Root { default void cleanUp() { /* do something 
> */
> } }
> interface FeatureTwo extends Root { default void cleanUp() { /* do something 
> */
> } }
> 
> My proxy implements both feature one and two, but in your API, there is no way
> for my `cleanUp` to call both `FeatureOne.super.cleanUp();` and
> `FeatureTwo.super.cleanUp();`. You should probably expose the lookup in your
> linker too, especially that you enabled nest access and you can just use that
> lookup to resolve, say, an implementation static method in the proxy user
> class.

yes, you are right, i should send the Lookup too.

> 
> Also the "delegate" in your API would significantly benefit from
> https://bugs.openjdk.java.net/browse/JDK-8282798 (#7744), too.

I don't think i need the carrier API, the idea is to have only one field in the 
proxy, this field can be a value type (exactly a primitive class) in the 
future, so being expanded into multiple fields by the VM at runtime.

> 
> -
> 
> PR: https://git.openjdk.java.net/jdk/pull/8278


Re: RFR: JDK-8242888: Convert dynamic proxy to hidden classes

2022-04-18 Thread liach
On Mon, 18 Apr 2022 20:42:48 GMT, Remi Forax  wrote:

> The third parameter of defineProxy() is a lambda which is called for each 
> method that can be overriden, either toString/equals/hashCode but also any 
> default methods,
if the lambda return true, the method is overriden, otherwise the default 
implementation is used.

Not quite, I mean calling default implementations in the super interface, 
consider:

interface Root { void cleanUp(); }
interface FeatureOne extends Root { default void cleanUp() { /* do something */ 
} }
interface FeatureTwo extends Root { default void cleanUp() { /* do something */ 
} }

My proxy implements both feature one and two, but in your API, there is no way 
for my `cleanUp` to call both `FeatureOne.super.cleanUp();` and 
`FeatureTwo.super.cleanUp();`. You should probably expose the lookup in your 
linker too, especially that you enabled nest access and you can just use that 
lookup to resolve, say, an implementation static method in the proxy user class.

Also the "delegate" in your API would significantly benefit from 
https://bugs.openjdk.java.net/browse/JDK-8282798 (#7744), too.

-

PR: https://git.openjdk.java.net/jdk/pull/8278


Re: RFR: JDK-8242888: Convert dynamic proxy to hidden classes

2022-04-18 Thread Remi Forax
- Original Message -
> From: "liach" 
> To: "core-libs-dev" , "security-dev" 
> 
> Sent: Monday, April 18, 2022 10:01:39 PM
> Subject: Re: RFR: JDK-8242888: Convert dynamic proxy to hidden classes

> On Sun, 17 Apr 2022 16:17:30 GMT, liach  wrote:
> 
>> Convert dynamic proxies to hidden classes. Modifies the serialization of 
>> proxies
>> (requires change in "Java Object Serialization Specification"). Makes the
>> proxies hidden in stack traces. Removes duplicate logic in proxy building.
>> 
>> The main compatibility changes and their rationales are:
>> 1. Modification to the serialization specification: In the "An instance of 
>> the
>> class is allocated... The contents restored appropriately" section, I propose
>> explicitly state that handling of proxies are unspecified as to allow
>> implementation freedom, though I've seen deliberate attempts for proxies to
>> implement interfaces with `readResolve` in order to control their 
>> serialization
>> behavior.
>>- This is for the existing generated constructor accessor is 
>> bytecode-based,
>>which cannot refer to hidden classes.
>>- An alternative is to preserve the behavior, where the serialization
>>constructor calls `invokespecial` on the closest serializable superclass'
>>no-arg constructor, like in #1830 by @DasBrain.
>>- My rationale against preservation is such super calls are unsafe and 
>> should be
>>discouraged in the long term. Calling the existing constructor with a 
>> dummy
>>argument, as in my implementation, would be more safe.
>> 2. The disappearance of proxies in stack traces.
>>- Same behavior exists in lambda expressions: For instance, in 
>> `((Runnable) ()
>>-> { throw new Error(); }).run();`, the `run` method, implemented by the
>>lambda, will not appear in the stack trace, and isn't too problematic.
>> 
>> A summary of the rest of the changes:
>> 1. Merged the two passes of determining module and package of the proxy into
>> one. This reduced a lot of code and allowed anchor class (for hidden class
>> creation) selection be done together as well.
>> 2. Exposed internal API for obtaining a full-privileged lookup to the rest of
>> `java.base`. This API is intended for implementation of legacy (pre
>> `MethodHandles.Lookup`) caller sensitive public APIs so they don't need more
>> complex tricks to obtain proper permissions as lookups.
>> 3. Implements [8229959](https://bugs.openjdk.java.net/browse/JDK-8229959):
>> passes methods computed by proxy generator as class data to the hidden proxy
>> class to reduce generated proxy class size and improve performance.
>> 
>> In addition, since this change is somewhat large, should we keep the old 
>> proxy
>> generator as well and have it toggled through a command-line flag (like the 
>> old
>> v49 proxy generator or the old reflection implementation)?
>> 
>> Please feel free to comment or review. This change definitely requires a CSR,
>> but I have yet to determine what specifications should be changed.
> 
> I strongly recommend a new API over revamping `Proxy` itself.
> https://github.com/forax/hidden_proxy would be a good prototype that serves
> most needs of the current Proxy API (except a few, like invoking default
> superinterface method). 

The third parameter of defineProxy() is a lambda which is called for each 
method that can be overriden, either toString/equals/hashCode but also any 
default methods,
if the lambda return true, the method is overriden, otherwise the default 
implementation is used.

> With hidden classes, I don't see much value in leaving
> the new API's produced instance in separate modules; what we have for hidden
> classes should be fine for the most part.
> 
> Imo the main obstacle is still the handling of serialization. The annotations
> are serializable, but currently hidden classes do not work with serialization
> at all and must use `writeReplace` and `readResolve`. And how to migrate
> annotations off proxies without breaking serialization is still a question as
> well. Maybe we can upgrade invocation handlers to allow them to declare custom
> `readResolve` logic for the proxy to facilitate migration away. How the new 
> API
> will treat serialization is still undetermined.

yes, i suppose that like with lambdas, we need a special overload of 
defineProxy that automatically implements writeReplace() and use a specific 
class SerializableProxy (mirroring how SerializableLambda works).

> 
> -
> 
> PR: https://git.openjdk.java.net/jdk/pull/8278


Re: RFR: JDK-8242888: Convert dynamic proxy to hidden classes

2022-04-18 Thread liach
On Sun, 17 Apr 2022 16:17:30 GMT, liach  wrote:

> Convert dynamic proxies to hidden classes. Modifies the serialization of 
> proxies (requires change in "Java Object Serialization Specification"). Makes 
> the proxies hidden in stack traces. Removes duplicate logic in proxy building.
> 
> The main compatibility changes and their rationales are:
> 1. Modification to the serialization specification: In the "An instance of 
> the class is allocated... The contents restored appropriately" section, I 
> propose explicitly state that handling of proxies are unspecified as to allow 
> implementation freedom, though I've seen deliberate attempts for proxies to 
> implement interfaces with `readResolve` in order to control their 
> serialization behavior.
>- This is for the existing generated constructor accessor is 
> bytecode-based, which cannot refer to hidden classes.
>- An alternative is to preserve the behavior, where the serialization 
> constructor calls `invokespecial` on the closest serializable superclass' 
> no-arg constructor, like in #1830 by @DasBrain.
>- My rationale against preservation is such super calls are unsafe and 
> should be discouraged in the long term. Calling the existing constructor with 
> a dummy argument, as in my implementation, would be more safe.
> 2. The disappearance of proxies in stack traces.
>- Same behavior exists in lambda expressions: For instance, in 
> `((Runnable) () -> { throw new Error(); }).run();`, the `run` method, 
> implemented by the lambda, will not appear in the stack trace, and isn't too 
> problematic.
> 
> A summary of the rest of the changes:
> 1. Merged the two passes of determining module and package of the proxy into 
> one. This reduced a lot of code and allowed anchor class (for hidden class 
> creation) selection be done together as well.
> 2. Exposed internal API for obtaining a full-privileged lookup to the rest of 
> `java.base`. This API is intended for implementation of legacy (pre 
> `MethodHandles.Lookup`) caller sensitive public APIs so they don't need more 
> complex tricks to obtain proper permissions as lookups.
> 3. Implements [8229959](https://bugs.openjdk.java.net/browse/JDK-8229959): 
> passes methods computed by proxy generator as class data to the hidden proxy 
> class to reduce generated proxy class size and improve performance.
> 
> In addition, since this change is somewhat large, should we keep the old 
> proxy generator as well and have it toggled through a command-line flag (like 
> the old v49 proxy generator or the old reflection implementation)?
> 
> Please feel free to comment or review. This change definitely requires a CSR, 
> but I have yet to determine what specifications should be changed.

I strongly recommend a new API over revamping `Proxy` itself. 
https://github.com/forax/hidden_proxy would be a good prototype that serves 
most needs of the current Proxy API (except a few, like invoking default 
superinterface method). With hidden classes, I don't see much value in leaving 
the new API's produced instance in separate modules; what we have for hidden 
classes should be fine for the most part.

Imo the main obstacle is still the handling of serialization. The annotations 
are serializable, but currently hidden classes do not work with serialization 
at all and must use `writeReplace` and `readResolve`. And how to migrate 
annotations off proxies without breaking serialization is still a question as 
well. Maybe we can upgrade invocation handlers to allow them to declare custom 
`readResolve` logic for the proxy to facilitate migration away. How the new API 
will treat serialization is still undetermined.

-

PR: https://git.openjdk.java.net/jdk/pull/8278


Re: RFR: JDK-8242888: Convert dynamic proxy to hidden classes

2022-04-18 Thread Mandy Chung
On Sun, 17 Apr 2022 16:17:30 GMT, liach  wrote:

> Convert dynamic proxies to hidden classes. Modifies the serialization of 
> proxies (requires change in "Java Object Serialization Specification"). Makes 
> the proxies hidden in stack traces. Removes duplicate logic in proxy building.
> 
> The main compatibility changes and their rationales are:
> 1. Modification to the serialization specification: In the "An instance of 
> the class is allocated... The contents restored appropriately" section, I 
> propose explicitly state that handling of proxies are unspecified as to allow 
> implementation freedom, though I've seen deliberate attempts for proxies to 
> implement interfaces with `readResolve` in order to control their 
> serialization behavior.
>- This is for the existing generated constructor accessor is 
> bytecode-based, which cannot refer to hidden classes.
>- An alternative is to preserve the behavior, where the serialization 
> constructor calls `invokespecial` on the closest serializable superclass' 
> no-arg constructor, like in #1830 by @DasBrain.
>- My rationale against preservation is such super calls are unsafe and 
> should be discouraged in the long term. Calling the existing constructor with 
> a dummy argument, as in my implementation, would be more safe.
> 2. The disappearance of proxies in stack traces.
>- Same behavior exists in lambda expressions: For instance, in 
> `((Runnable) () -> { throw new Error(); }).run();`, the `run` method, 
> implemented by the lambda, will not appear in the stack trace, and isn't too 
> problematic.
> 
> A summary of the rest of the changes:
> 1. Merged the two passes of determining module and package of the proxy into 
> one. This reduced a lot of code and allowed anchor class (for hidden class 
> creation) selection be done together as well.
> 2. Exposed internal API for obtaining a full-privileged lookup to the rest of 
> `java.base`. This API is intended for implementation of legacy (pre 
> `MethodHandles.Lookup`) caller sensitive public APIs so they don't need more 
> complex tricks to obtain proper permissions as lookups.
> 3. Implements [8229959](https://bugs.openjdk.java.net/browse/JDK-8229959): 
> passes methods computed by proxy generator as class data to the hidden proxy 
> class to reduce generated proxy class size and improve performance.
> 
> In addition, since this change is somewhat large, should we keep the old 
> proxy generator as well and have it toggled through a command-line flag (like 
> the old v49 proxy generator or the old reflection implementation)?
> 
> Please feel free to comment or review. This change definitely requires a CSR, 
> but I have yet to determine what specifications should be changed.

It's good to see more experiment and prototype for this issue.   Like Alan 
said, the spec change, compatibility risks and security issues in particular on 
the serialization spec/impl change require lot of discussions and also 
prototyping of other alternatives.   A new API may also be one alternative to 
consider.

The current spec of Proxy class is defined with null protection domain (same 
protection domain as the bootstrap class loader). Lookup::defineHiddenClass 
defines the hidden class in the same protection domain as the defining class 
loader.   This would become a non-issue when the security manager is removed.  
However, before the removal of security manager, we still need to look into the 
compatibility risks and what and how applications/libraries depend on this 
behavior. 

During the development of JEP 371, I had a prototype of converting dynamic 
proxies to hidden class by adding a shim class (that's what your prototype 
does).   That raises the question "how to get a Lookup on a package for the 
dynamic proxy class to be injected in without injecting a shim class (i.e. your 
anchor class)?"  This functionality could be considered as a separate RFE.
However, frameworks don't always have the access to inject a class in a package 
defined to a class loader.  This is something worth looking into.

-

PR: https://git.openjdk.java.net/jdk/pull/8278


Re: RFR: JDK-8242888: Convert dynamic proxy to hidden classes

2022-04-18 Thread Alan Bateman

On 17/04/2022 17:24, liach wrote:

Convert dynamic proxies to hidden classes. Modifies the serialization of proxies 
(requires change in "Java Object Serialization Specification"). Makes the 
proxies hidden in stack traces. Removes duplicate logic in proxy building.

The main compatibility changes and their rationales are:
1. Modification to the serialization specification: In the "An instance of the class 
is allocated... The contents restored appropriately" section, I propose explicitly 
state that handling of proxies are unspecified as to allow implementation freedom, though 
I've seen deliberate attempts for proxies to implement interfaces with `readResolve` in 
order to control their serialization behavior.
- This is for the existing generated constructor accessor is 
bytecode-based, which cannot refer to hidden classes.
- An alternative is to preserve the behavior, where the serialization 
constructor calls `invokespecial` on the closest serializable superclass' 
no-arg constructor, like in #1830 by @DasBrain.
- My rationale against preservation is such super calls are unsafe and 
should be discouraged in the long term. Calling the existing constructor with a 
dummy argument, as in my implementation, would be more safe.
2. The disappearance of proxies in stack traces.
- Same behavior exists in lambda expressions: For instance, in `((Runnable) () 
-> { throw new Error(); }).run();`, the `run` method, implemented by the 
lambda, will not appear in the stack trace, and isn't too problematic.


It's great that you have time to experiment in this area but just to set 
expectations that it will likely require significant discussion and 
maybe prototyping of alternatives. It suspect the Reviewer effort will 
end up being many times the effort required to do the actual work 
because of the compatibility and security issues that will need to be 
worked through.


I think you need to add non-discoverability to the list of compatibility 
issues. Do we know for sure that there aren't frameworks and libraries 
that use Class.forName on proxy classes? We've had issues in the past 
with changes in this area.


It's too early to say but it might be that the compatibility risks may 
nudge this one into creating a new API.


-Alan.





Re: RFR: JDK-8242888: Convert dynamic proxy to hidden classes

2022-04-17 Thread liach
On Sun, 17 Apr 2022 16:17:30 GMT, liach  wrote:

> Convert dynamic proxies to hidden classes. Modifies the serialization of 
> proxies (requires change in "Java Object Serialization Specification"). Makes 
> the proxies hidden in stack traces. Removes duplicate logic in proxy building.
> 
> The main compatibility changes and their rationales are:
> 1. Modification to the serialization specification: In the "An instance of 
> the class is allocated... The contents restored appropriately" section, I 
> propose explicitly state that handling of proxies are unspecified as to allow 
> implementation freedom, though I've seen deliberate attempts for proxies to 
> implement interfaces with `readResolve` in order to control their 
> serialization behavior.
>- This is for the existing generated constructor accessor is 
> bytecode-based, which cannot refer to hidden classes.
>- An alternative is to preserve the behavior, where the serialization 
> constructor calls `invokespecial` on the closest serializable superclass' 
> no-arg constructor, like in #1830 by @DasBrain.
>- My rationale against preservation is such super calls are unsafe and 
> should be discouraged in the long term. Calling the existing constructor with 
> a dummy argument, as in my implementation, would be more safe.
> 2. The disappearance of proxies in stack traces.
>- Same behavior exists in lambda expressions: For instance, in 
> `((Runnable) () -> { throw new Error(); }).run();`, the `run` method, 
> implemented by the lambda, will not appear in the stack trace, and isn't too 
> problematic.
> 
> A summary of the rest of the changes:
> 1. Merged the two passes of determining module and package of the proxy into 
> one. This reduced a lot of code and allowed anchor class (for hidden class 
> creation) selection be done together as well.
> 2. Exposed internal API for obtaining a full-privileged lookup to the rest of 
> `java.base`. This API is intended for implementation of legacy (pre 
> `MethodHandles.Lookup`) caller sensitive public APIs so they don't need more 
> complex tricks to obtain proper permissions as lookups.
> 3. Implements [8229959](https://bugs.openjdk.java.net/browse/JDK-8229959): 
> passes methods computed by proxy generator as class data to the hidden proxy 
> class to reduce generated proxy class size and improve performance.
> 
> In addition, since this change is somewhat large, should we keep the old 
> proxy generator as well and have it toggled through a command-line flag (like 
> the old v49 proxy generator or the old reflection implementation)?
> 
> Please feel free to comment or review. This change definitely requires a CSR, 
> but I have yet to determine what specifications should be changed.

I now think that this should be split into probably four separate patches:
1. Merge the 2 passes of scanning interfaces, and store the 
exported/non-exported package within the classloader value info for dynamic 
modules
2. Use the JLIA access to obtain lookup than spin an accessor in every proxy 
class
3. Update serialization constructor generation to use method handles (as in 
#1830) for hidden classes, don't change the serialization specs for now, as it 
affects security.

And finally, convert proxies to hidden classes like in this pr. There will be 
an option (most likely command line) to restore old proxies. And in the new 
proxies, method objects can be passed as class data, like in the current patch. 
The jshell test change will be in the final conversion patch.

-

PR: https://git.openjdk.java.net/jdk/pull/8278


Re: RFR: JDK-8242888: Convert dynamic proxy to hidden classes

2022-04-17 Thread Peter Firmstone
We re-implemented a subset of Java Serialization, prior to the creation 
Java serialization filters.  Field types are read ahead from the stream 
and invariant's validated during construction for failure atomicity (a 
special constructor signature is used for deserialization), there are 
also stream limits that require periodical stream resets, to avoid 
million laugh style attacks. Also the source of the serialized data must 
be authenticated before permission is granted for parsing serial data.   
We also removed the ability to deserialize object graphs with circular 
links.


This is used for an Remove invocation framework called JERI (Jini 
extensible remote invocation).  Serialization has been given a public 
API to allow extensibility, that is to allow other serialization 
protocols to be used via configuration, using a Serialization layer.


Service and service discovery architecture makes use of JERI, for 
marshaling object parameters securely over the network, for remote 
method calls on services, via remote proxy's.


The way that JERI serializes proxy's has changed significantly since 
Jini, instead of marshaling proxy's, a bootstrap proxy (local code only) 
is marshaled first, the client first authenticates the connection, the 
bootstrap proxy is used to provide information for dynamic downloading 
of any required jar files, or wiring of dependencies prior to the 
unmarshaling of the service proxy.  Unlike Java RMI, which uses 
RMIClassLoading for class resolution, each Jeri Endpoint is assigned a 
ClassLoader for class resolution during deserialization / object 
unmarshaling, this solves the OSGi deserialzation class resolution problem.


https://github.com/pfirmstone/JGDMS/blob/trunk/JGDMS/jgdms-platform/src/main/java/org/apache/river/api/io/ProxySerializer.java

https://github.com/pfirmstone/JGDMS/blob/c1edf5892306f24f8f97f459f499dec54984b08f/JGDMS/jgdms-platform/src/main/java/org/apache/river/api/io/AtomicMarshalInputStream.java#L2263

https://github.com/pfirmstone/JGDMS/blob/c1edf5892306f24f8f97f459f499dec54984b08f/JGDMS/jgdms-platform/src/main/java/org/apache/river/api/io/AtomicMarshalInputStream.java#L853

Would be nice to do some testing with the changes.

--
Regards,
 
Peter Firmstone

0498 286 363
Zeus Project Services Pty Ltd.

On 18/04/2022 2:24 am, liach wrote:

Convert dynamic proxies to hidden classes. Modifies the serialization of proxies 
(requires change in "Java Object Serialization Specification"). Makes the 
proxies hidden in stack traces. Removes duplicate logic in proxy building.

The main compatibility changes and their rationales are:
1. Modification to the serialization specification: In the "An instance of the class 
is allocated... The contents restored appropriately" section, I propose explicitly 
state that handling of proxies are unspecified as to allow implementation freedom, though 
I've seen deliberate attempts for proxies to implement interfaces with `readResolve` in 
order to control their serialization behavior.
- This is for the existing generated constructor accessor is 
bytecode-based, which cannot refer to hidden classes.
- An alternative is to preserve the behavior, where the serialization 
constructor calls `invokespecial` on the closest serializable superclass' 
no-arg constructor, like in #1830 by @DasBrain.
- My rationale against preservation is such super calls are unsafe and 
should be discouraged in the long term. Calling the existing constructor with a 
dummy argument, as in my implementation, would be more safe.
2. The disappearance of proxies in stack traces.
- Same behavior exists in lambda expressions: For instance, in `((Runnable) () 
-> { throw new Error(); }).run();`, the `run` method, implemented by the 
lambda, will not appear in the stack trace, and isn't too problematic.

A summary of the rest of the changes:
1. Merged the two passes of determining module and package of the proxy into 
one. This reduced a lot of code and allowed anchor class (for hidden class 
creation) selection be done together as well.
2. Exposed internal API for obtaining a full-privileged lookup to the rest of 
`java.base`. This API is intended for implementation of legacy (pre 
`MethodHandles.Lookup`) caller sensitive public APIs so they don't need more 
complex tricks to obtain proper permissions as lookups.
3. Implements [8229959](https://bugs.openjdk.java.net/browse/JDK-8229959): 
passes methods computed by proxy generator as class data to the hidden proxy 
class to reduce generated proxy class size and improve performance.

In addition, since this change is somewhat large, should we keep the old proxy 
generator as well and have it toggled through a command-line flag (like the old 
v49 proxy generator or the old reflection implementation)?

Please feel free to comment or review. This change definitely requires a CSR, 
but I have yet to determine what specifications should be changed.

-

Commit messages:
  - Change proxy