On 27 March 2017 at 11:21, Philippe Mouawad <[email protected]> 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 <[email protected]> wrote: > >> On 26 March 2017 at 21:00, <[email protected]> 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.
