jmeter-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From sebb <seb...@gmail.com>
Subject Re: svn commit: r1623759 - /jmeter/trunk/src/core/org/apache/jmeter/gui/tree/JMeterTreeNode.java
Date Sat, 13 Sep 2014 13:12:51 GMT
 On 13 September 2014 13:51, Philippe Mouawad
<philippe.mouawad@gmail.com> wrote:
> Regarding sebb note after my removal of final.
>
> @Sebb, can you explain why we need final here ? I don't get it.
> Reading literature, having final transient for non static field does not
> seems to be a valid thing except for compile-time *constant expression*
> (ยง15.28) of http://docs.oracle.com/javase/specs/jls/se5.0/jls3.pdf.
>
> If I am wrong, can you point me to the section of spec or reference article
> that says we should have here final transient ?
>
> I agree with you it will work wether or not we put it but as per specs it
> seems wrong to put final.

The point here is that we don't actually want to (de)serialise the class.
It's not used in client-server mode which is where JMeter uses serialisation.
The class is only Serializable because it inherits from DefaultMutableTreeNode.

The original code used a non-constant final field, which would not
work if JMeter used serialisation.
Yet JMeter worked in most JVMs, because JMeter did not need to serialise it.
We used final because the field is immutable, and final helps ensure
thread-safety.

Now it appears that MacOS Java tries to use serialisation when using copy-paste.
So we had to add transient to avoid that feature/bug.
But that is a work-round for that particular JVM, and should not
require use to change code that is otherwise working.

> I am always keen on learning new things so will be happy here to understand
> what's wrong.
> Thanks
> Regards
> Philippe
>
>
> On Sat, Sep 13, 2014 at 2:25 PM, Philippe Mouawad <
> philippe.mouawad@gmail.com> wrote:
>
>> I think removing final in this case seems better according to literature
>> on the subject, although keeping it wouldn't break anything due to how
>> JMeter uses this class.
>> So I reverted to previous state, ie only keep transient.
>>
>> Regards
>>
>>
>> On Wed, Sep 10, 2014 at 1:08 PM, sebb <sebbaz@gmail.com> wrote:
>>
>>> On 10 September 2014 07:19, Philippe Mouawad <philippe.mouawad@gmail.com>
>>> wrote:
>>> > well that's what I initially thought, first commit removed it.
>>> > Then sebb made a comment saying why I removed final.
>>> > I added it back to see if it worked , and it did.
>>> >
>>> > Although reading litterature it was said it would be an issue during
>>> > deserialization.
>>> >
>>> > According to this it seems you are right:
>>> >
>>> http://blog.clempinch.com/transient-and-final-instance-variables-in-java/
>>> >
>>> > I think it works thougj because we rebuild an object from the
>>> deserialized
>>> > one.
>>>
>>> In this case we could not use standard deserialisation techniques with
>>> the original non-transient final field anyway.
>>>
>>> That is one of the problems of serialisation - it's hard to use it on
>>> immutable fields which are built in their constructors.
>>> Adding the transient marker just stops the field from being serialised.
>>>
>>> AFAICT a non-constant final field cannot be serialised, regardless of
>>> the transient marker.
>>>
>>> > But your analysis and a ref article on this would help me learn new
>>> things
>>> > and have a definite position on this.
>>> >
>>> > Thanks for checking
>>> >
>>> > On Wednesday, September 10, 2014, Felix Schumacher <
>>> > felix.schumacher@internetallee.de
>>> > <javascript:_e(%7B%7D,'cvml','felix.schumacher@internetallee.de');>>
>>> wrote:
>>> >
>>> >>
>>> >>
>>> >> On 9. September 2014 13:25:39 MESZ, pmouawad@apache.org wrote:
>>> >> >Author: pmouawad
>>> >> >Date: Tue Sep  9 11:25:38 2014
>>> >> >New Revision: 1623759
>>> >> >
>>> >> >URL: http://svn.apache.org/r1623759
>>> >> >Log:
>>> >> >Bug 54648 - JMeter GUI on OS X crashes when using CMD+C (keyboard
>>> >> >shortcut or UI menu entry) on an element from the tree
>>> >> >Put back final.
>>> >> >Bugzilla Id: 54648
>>> >> >
>>> >> >Modified:
>>> >> >
>>>  jmeter/trunk/src/core/org/apache/jmeter/gui/tree/JMeterTreeNode.java
>>> >> >
>>> >> >Modified:
>>> >> >jmeter/trunk/src/core/org/apache/jmeter/gui/tree/JMeterTreeNode.java
>>> >> >URL:
>>> >> >
>>> >>
>>> http://svn.apache.org/viewvc/jmeter/trunk/src/core/org/apache/jmeter/gui/tree/JMeterTreeNode.java?rev=1623759&r1=1623758&r2=1623759&view=diff
>>> >>
>>> >>
>>> >==============================================================================
>>> >> >---
>>> >> >jmeter/trunk/src/core/org/apache/jmeter/gui/tree/JMeterTreeNode.java
>>> >> >(original)
>>> >> >+++
>>> >> >jmeter/trunk/src/core/org/apache/jmeter/gui/tree/JMeterTreeNode.java
>>> >> >Tue Sep  9 11:25:38 2014
>>> >> >@@ -47,7 +47,7 @@ public class JMeterTreeNode extends Defa
>>> >> >     private static final int TEST_PLAN_LEVEL = 1;
>>> >> >
>>> >> >     // See Bug 54648
>>> >> >-    private transient JMeterTreeModel treeModel;
>>> >> >+    private final transient JMeterTreeModel treeModel;
>>> >> I don't believe that fields can be transient and final at the same
>>> time.
>>> >>
>>> >> Since transient is for serialization and final fields have to be set
>>> >> during construction. But deserialization will not call the constructor.
>>> >>
>>> >> Regards
>>> >> Felix
>>> >> >
>>> >> >     private boolean markedBySearch;
>>> >> >
>>> >>
>>> >>
>>> >
>>> > --
>>> > Cordialement.
>>> > Philippe Mouawad.
>>>
>>
>>
>>
>> --
>> Cordialement.
>> Philippe Mouawad.
>>
>>
>>
>
>
> --
> Cordialement.
> Philippe Mouawad.

Mime
View raw message