Tapestry 5.5-beta-2 problem: @Parameter field access problem with anonymous classes

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

Tapestry 5.5-beta-2 problem: @Parameter field access problem with anonymous classes

Dmitry Gusev
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
Reply | Threaded
Open this post in threaded view
|

Re: Tapestry 5.5-beta-2 problem: @Parameter field access problem with anonymous classes

Dmitry Gusev
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 <[hidden email]> 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
Reply | Threaded
Open this post in threaded view
|

Re: Tapestry 5.5-beta-2 problem: @Parameter field access problem with anonymous classes

Dmitry Gusev
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 <[hidden email]>
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 <[hidden email]>
> 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