You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@jmeter.apache.org by sebb <se...@gmail.com> on 2017/03/26 23:25:02 UTC
Re: svn commit: r1788774 - /jmeter/trunk/src/core/org/apache/jmeter/samplers/SampleSaveConfiguration.java
On 26 March 2017 at 21:00, <pm...@apache.org> wrote:
> Author: pmouawad
> Date: Sun Mar 26 20:00:59 2017
> New Revision: 1788774
>
> URL: http://svn.apache.org/viewvc?rev=1788774&view=rev
> Log:
> Bug 60830 Timestamps in CSV file could be corrupted due to sharing a SimpleDateFormatter across threads
> Bugzilla Id: 60830
>
> Modified:
> jmeter/trunk/src/core/org/apache/jmeter/samplers/SampleSaveConfiguration.java
>
> Modified: jmeter/trunk/src/core/org/apache/jmeter/samplers/SampleSaveConfiguration.java
> URL: http://svn.apache.org/viewvc/jmeter/trunk/src/core/org/apache/jmeter/samplers/SampleSaveConfiguration.java?rev=1788774&r1=1788773&r2=1788774&view=diff
> ==============================================================================
> --- jmeter/trunk/src/core/org/apache/jmeter/samplers/SampleSaveConfiguration.java (original)
> +++ jmeter/trunk/src/core/org/apache/jmeter/samplers/SampleSaveConfiguration.java Sun Mar 26 20:00:59 2017
> @@ -475,11 +475,10 @@ public class SampleSaveConfiguration imp
> // Does not appear to be used (yet)
> private int assertionsResultsToSave = ASSERTIONS_RESULT_TO_SAVE;
>
> -
> // Don't save this, as it is derived from the time format
> private boolean printMilliseconds = PRINT_MILLISECONDS;
>
> - private String dateFormat = DATE_FORMAT;
> + private transient String dateFormat = DATE_FORMAT;
Why is this transient?
If the class is deserialised, how does the field get set up?
> /** A formatter for the time stamp.
> * Make transient as we don't want to save the FastDateFormat class
> @@ -613,7 +612,7 @@ public class SampleSaveConfiguration imp
> try {
> SampleSaveConfiguration clone = (SampleSaveConfiguration)super.clone();
> if(this.dateFormat != null) {
> - clone.threadSafeLenientFormatter = (FastDateFormat)this.threadSafeLenientFormatter.clone();
> + clone.threadSafeLenientFormatter = (FastDateFormat)this.threadSafeLenientFormatter().clone();
> }
> return clone;
> }
> @@ -970,6 +969,12 @@ public class SampleSaveConfiguration imp
> * @return {@link FastDateFormat} Thread safe lenient formatter
> */
> public FastDateFormat threadSafeLenientFormatter() {
> + // When restored by XStream threadSafeLenientFormatter may not have
> + // been initialized
> + if(threadSafeLenientFormatter == null) {
> + threadSafeLenientFormatter =
> + dateFormat != null ? FastDateFormat.getInstance(dateFormat) : null;
> + }
> return threadSafeLenientFormatter;
> }
>
>
>
Re: svn commit: r1788774 - /jmeter/trunk/src/core/org/apache/jmeter/samplers/SampleSaveConfiguration.java
Posted by sebb <se...@gmail.com>.
On 27 March 2017 at 11:21, Philippe Mouawad <ph...@gmail.com> wrote:
> Hi sebb,
> See readResolve which initializes it.
>
> Note I made it transient because I wrongly didn't do it initially.
That's what confused me.
The commit fixed two things but only one was mentioned in the log.
Best to restrict commits to a single issue at a time.
Or at least ensure that the log message mentions both fixes.
> This led it to be persisted in XML and when reloaded, it would be
> initialized through XStream unmarshalling (properties access) while
> threadSafeLenientFormatter would not, which lead to NPE in clone() when
> reloading the XML.
>
> So I did 2 things:
> - Made it transient
> - Used threadSafeLenientFormatter() in clone and modified
> threadSafeLenientFormatter() to initialize threadSafeLenientFormatter
>
> Regards
> Philippe
>
>
> On Mon, Mar 27, 2017 at 1:25 AM, sebb <se...@gmail.com> wrote:
>
>> On 26 March 2017 at 21:00, <pm...@apache.org> wrote:
>> > Author: pmouawad
>> > Date: Sun Mar 26 20:00:59 2017
>> > New Revision: 1788774
>> >
>> > URL: http://svn.apache.org/viewvc?rev=1788774&view=rev
>> > Log:
>> > Bug 60830 Timestamps in CSV file could be corrupted due to sharing a
>> SimpleDateFormatter across threads
>> > Bugzilla Id: 60830
>> >
>> > Modified:
>> > jmeter/trunk/src/core/org/apache/jmeter/samplers/SampleSave
>> Configuration.java
>> >
>> > Modified: jmeter/trunk/src/core/org/apache/jmeter/samplers/SampleSaveC
>> onfiguration.java
>> > URL: http://svn.apache.org/viewvc/jmeter/trunk/src/core/org/apach
>> e/jmeter/samplers/SampleSaveConfiguration.java?rev=1788774&r
>> 1=1788773&r2=1788774&view=diff
>> > ============================================================
>> ==================
>> > --- jmeter/trunk/src/core/org/apache/jmeter/samplers/SampleSaveConfiguration.java
>> (original)
>> > +++ jmeter/trunk/src/core/org/apache/jmeter/samplers/SampleSaveConfiguration.java
>> Sun Mar 26 20:00:59 2017
>> > @@ -475,11 +475,10 @@ public class SampleSaveConfiguration imp
>> > // Does not appear to be used (yet)
>> > private int assertionsResultsToSave = ASSERTIONS_RESULT_TO_SAVE;
>> >
>> > -
>> > // Don't save this, as it is derived from the time format
>> > private boolean printMilliseconds = PRINT_MILLISECONDS;
>> >
>> > - private String dateFormat = DATE_FORMAT;
>> > + private transient String dateFormat = DATE_FORMAT;
>>
>> Why is this transient?
>> If the class is deserialised, how does the field get set up?
>>
>> > /** A formatter for the time stamp.
>> > * Make transient as we don't want to save the FastDateFormat class
>> > @@ -613,7 +612,7 @@ public class SampleSaveConfiguration imp
>> > try {
>> > SampleSaveConfiguration clone =
>> (SampleSaveConfiguration)super.clone();
>> > if(this.dateFormat != null) {
>> > - clone.threadSafeLenientFormatter =
>> (FastDateFormat)this.threadSafeLenientFormatter.clone();
>> > + clone.threadSafeLenientFormatter =
>> (FastDateFormat)this.threadSafeLenientFormatter().clone();
>> > }
>> > return clone;
>> > }
>> > @@ -970,6 +969,12 @@ public class SampleSaveConfiguration imp
>> > * @return {@link FastDateFormat} Thread safe lenient formatter
>> > */
>> > public FastDateFormat threadSafeLenientFormatter() {
>> > + // When restored by XStream threadSafeLenientFormatter may not
>> have
>> > + // been initialized
>> > + if(threadSafeLenientFormatter == null) {
>> > + threadSafeLenientFormatter =
>> > + dateFormat != null ? FastDateFormat.getInstance(dateFormat)
>> : null;
>> > + }
>> > return threadSafeLenientFormatter;
>> > }
>> >
>> >
>> >
>>
>
>
>
> --
> Cordialement.
> Philippe Mouawad.
Re: svn commit: r1788774 - /jmeter/trunk/src/core/org/apache/jmeter/samplers/SampleSaveConfiguration.java
Posted by Philippe Mouawad <ph...@gmail.com>.
Hi sebb,
See readResolve which initializes it.
Note I made it transient because I wrongly didn't do it initially.
This led it to be persisted in XML and when reloaded, it would be
initialized through XStream unmarshalling (properties access) while
threadSafeLenientFormatter would not, which lead to NPE in clone() when
reloading the XML.
So I did 2 things:
- Made it transient
- Used threadSafeLenientFormatter() in clone and modified
threadSafeLenientFormatter() to initialize threadSafeLenientFormatter
Regards
Philippe
On Mon, Mar 27, 2017 at 1:25 AM, sebb <se...@gmail.com> wrote:
> On 26 March 2017 at 21:00, <pm...@apache.org> wrote:
> > Author: pmouawad
> > Date: Sun Mar 26 20:00:59 2017
> > New Revision: 1788774
> >
> > URL: http://svn.apache.org/viewvc?rev=1788774&view=rev
> > Log:
> > Bug 60830 Timestamps in CSV file could be corrupted due to sharing a
> SimpleDateFormatter across threads
> > Bugzilla Id: 60830
> >
> > Modified:
> > jmeter/trunk/src/core/org/apache/jmeter/samplers/SampleSave
> Configuration.java
> >
> > Modified: jmeter/trunk/src/core/org/apache/jmeter/samplers/SampleSaveC
> onfiguration.java
> > URL: http://svn.apache.org/viewvc/jmeter/trunk/src/core/org/apach
> e/jmeter/samplers/SampleSaveConfiguration.java?rev=1788774&r
> 1=1788773&r2=1788774&view=diff
> > ============================================================
> ==================
> > --- jmeter/trunk/src/core/org/apache/jmeter/samplers/SampleSaveConfiguration.java
> (original)
> > +++ jmeter/trunk/src/core/org/apache/jmeter/samplers/SampleSaveConfiguration.java
> Sun Mar 26 20:00:59 2017
> > @@ -475,11 +475,10 @@ public class SampleSaveConfiguration imp
> > // Does not appear to be used (yet)
> > private int assertionsResultsToSave = ASSERTIONS_RESULT_TO_SAVE;
> >
> > -
> > // Don't save this, as it is derived from the time format
> > private boolean printMilliseconds = PRINT_MILLISECONDS;
> >
> > - private String dateFormat = DATE_FORMAT;
> > + private transient String dateFormat = DATE_FORMAT;
>
> Why is this transient?
> If the class is deserialised, how does the field get set up?
>
> > /** A formatter for the time stamp.
> > * Make transient as we don't want to save the FastDateFormat class
> > @@ -613,7 +612,7 @@ public class SampleSaveConfiguration imp
> > try {
> > SampleSaveConfiguration clone =
> (SampleSaveConfiguration)super.clone();
> > if(this.dateFormat != null) {
> > - clone.threadSafeLenientFormatter =
> (FastDateFormat)this.threadSafeLenientFormatter.clone();
> > + clone.threadSafeLenientFormatter =
> (FastDateFormat)this.threadSafeLenientFormatter().clone();
> > }
> > return clone;
> > }
> > @@ -970,6 +969,12 @@ public class SampleSaveConfiguration imp
> > * @return {@link FastDateFormat} Thread safe lenient formatter
> > */
> > public FastDateFormat threadSafeLenientFormatter() {
> > + // When restored by XStream threadSafeLenientFormatter may not
> have
> > + // been initialized
> > + if(threadSafeLenientFormatter == null) {
> > + threadSafeLenientFormatter =
> > + dateFormat != null ? FastDateFormat.getInstance(dateFormat)
> : null;
> > + }
> > return threadSafeLenientFormatter;
> > }
> >
> >
> >
>
--
Cordialement.
Philippe Mouawad.