tapestry-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Dmitry Gusev <dmitry.gu...@gmail.com>
Subject Re: Tapestry 5.5-beta-2 problem: @Parameter field access problem with anonymous classes
Date Wed, 24 Apr 2019 17:53:07 GMT
Hello,

It appears the problem is because Java 11 changed the contract for inner
classes to access state of outer classes:

https://www.baeldung.com/java-nest-based-access-control

Before Java 11 there were no direct field access instructions in inner
classes, instead java used synthetic bridge methods, like access$0001.
These accessor methods were created in outer classes and the methods were
actually exposing state of owning class as regular getters/setters.

Actual replacement of field access is done in the scope of top-level
classes by the PlasticClassImpl#interceptFieldAccess
<https://github.com/apache/tapestry-5/blob/d3928ad44714b949d247af2652c84dae3c27e1b1/plastic/src/main/java/org/apache/tapestry5/internal/plastic/PlasticClassImpl.java#L1233>
using
field instrumentations recorded by PlasticClassImpl#redirectFieldRead
<https://github.com/apache/tapestry-5/blob/d3928ad44714b949d247af2652c84dae3c27e1b1/plastic/src/main/java/org/apache/tapestry5/internal/plastic/PlasticClassImpl.java#L1616>
and PlasticClassImpl#redirectFieldWrite
<https://github.com/apache/tapestry-5/blob/d3928ad44714b949d247af2652c84dae3c27e1b1/plastic/src/main/java/org/apache/tapestry5/internal/plastic/PlasticClassImpl.java#L1604>
.

Inner classes are loaded differently (PlasticClassPool#loadInnerClass
<https://github.com/apache/tapestry-5/blob/d3928ad44714b949d247af2652c84dae3c27e1b1/plastic/src/main/java/org/apache/tapestry5/internal/plastic/PlasticClassPool.java#L388>),
they don't have access to the same field instrumentations and can only take
instrumentations from the PlasticClassPool, see
PlasticClassPool#interceptFieldAccess
<https://github.com/apache/tapestry-5/blob/d3928ad44714b949d247af2652c84dae3c27e1b1/plastic/src/main/java/org/apache/tapestry5/internal/plastic/PlasticClassPool.java#L405>
.

Although PlasticClassImpl can share field instrumentations with the pool,
it only shares instrumentations for the non-private fields of the non-proxy
classes
<https://github.com/apache/tapestry-5/blob/d3928ad44714b949d247af2652c84dae3c27e1b1/plastic/src/main/java/org/apache/tapestry5/internal/plastic/PlasticClassImpl.java#L1610>
.
Before java 11 this wasn't critical because of the bridge methods, but now
it is: nested classes, and classes declared in the same compilation unit
can still access private members of each other, hence they may need to have
access to the instrumentations.

A patch like below can fix the issue, but I'm not sure if there can be any
other side effects, i.e. why were the private field instrumentations not
shared with the pool before?

diff --git
a/plastic/src/main/java/org/apache/tapestry5/internal/plastic/PlasticClassImpl.java
b/plastic/src/main/java/org/apache/tapestry5/internal/plastic/PlasticClassImpl.java
index fffd910af..735952d32 100644
---
a/plastic/src/main/java/org/apache/tapestry5/internal/plastic/PlasticClassImpl.java
+++
b/plastic/src/main/java/org/apache/tapestry5/internal/plastic/PlasticClassImpl.java
@@ -1607,7 +1607,7 @@ public class PlasticClassImpl extends Lockable
implements PlasticClass, Internal

         fieldInstrumentations.write.put(fieldName, fi);

-        if (!(proxy || privateField))
+        if (!(proxy/* || privateField*/))
         {
             pool.setFieldWriteInstrumentation(classNode.name, fieldName,
fi);
         }
@@ -1619,7 +1619,7 @@ public class PlasticClassImpl extends Lockable
implements PlasticClass, Internal

         fieldInstrumentations.read.put(fieldName, fi);

-        if (!(proxy || privateField))
+        if (!(proxy/* || privateField*/))
         {
             pool.setFieldReadInstrumentation(classNode.name, fieldName,
fi);
         }


Best regards,
Dmitry





On Fri, Apr 12, 2019 at 11:41 AM Dmitry Gusev <dmitry.gusev@gmail.com>
wrote:

> I ran some more tests and found the problem only exists in Java 11 target
> compatibility, it works fine with Java 9 and 10.
>
> Could be a bug in ASM, am I right we're using ASM 7.0 (judging by
> plastic/LICENSE-ASM-7_0.txt)?
>
> There's version 7.1 released 3 March 2019 with some bug fixes, I haven't
> tried that yet.
>
>
> On Thu, Apr 11, 2019 at 9:29 PM Dmitry Gusev <dmitry.gusev@gmail.com>
> wrote:
>
>> Hi team!
>>
>> Just wanted to share an issue I found trying to upgrade to 5.5-beta-2 and
>> Java 11, maybe anyone have seen anything similar before.
>>
>> In our pages/components it's quite common to build anonymous classes for
>> Grid data sources, i.e. say if we have a <t:Grid source="sourceObject"> the
>> sourceObject can be defined as anonymous class that's referencing some
>> parameter object:
>>
>> @Parameter
>> private Object parameterObject1;
>>
>> public GridDataSource getSourceObject()
>> {
>>     return new GridDataSource()
>>     {
>>         @Override
>>         public int getAvailableRows()
>>         {
>>             return count(parameterObject1); // this does not read from
>> parameter conduit
>>         }
>>     };
>> }
>>
>> I've found that the read access for the parameterObject1 is not replaced
>> in this case, and parameterObject1 reads its value from a field instead of
>> a conduit.
>>
>> However, if parameter access is factored out from the anonymous class -
>> the value is read as expected, i.e. this works fine:
>>
>> public GridDataSource getSourceObject()
>> {
>>     return new MyDataSource(parameterObject1); // this reads the value
>> from conduit as expected
>> }
>>
>> public class MyDataSource implements GridDataSource
>> {
>>     private final Object valueOfParameterObject1;
>>
>>     public MyDataSource(Object parameterObject1)
>>     {
>>         this.valueOfParameterObject1 = parameterObject1;
>>     }
>>
>>     @Override
>>     public int getAvailableRows()
>>     {
>>         return count(valueOfParameterObject1);
>>     }
>> }
>>
>> I haven't dig any further yet, but assume it can be related with some
>> changes in ASM / byte code.
>>
>> Just thought I'd ask here before I continue.
>>
>> Best regards,
>> Dmitry
>>
>
>
> --
> Dmitry Gusev
>
> AnjLab Team
> http://anjlab.com
>


-- 
Dmitry Gusev

AnjLab Team
http://anjlab.com

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message