Bug 60818 - Output of CSV files is not threadsafe
Summary: Output of CSV files is not threadsafe
Status: RESOLVED DUPLICATE of bug 60830
Alias: None
Product: JMeter
Classification: Unclassified
Component: Main (show other bugs)
Version: 3.1
Hardware: All All
: P2 normal (vote)
Target Milestone: ---
Assignee: JMeter issues mailing list
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-03-04 10:37 UTC by Felix Schumacher
Modified: 2017-03-08 20:40 UTC (History)
1 user (show)



Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Felix Schumacher 2017-03-04 10:37:19 UTC
The printing to CSV is not threadsafe and may lead to garbled CSV files, which cannot be read in again.
Comment 1 Felix Schumacher 2017-03-04 10:57:52 UTC
Date: Sat Mar  4 10:55:15 2017
New Revision: 1785464

URL: http://svn.apache.org/viewvc?rev=1785464&view=rev
Log:
Synchronize file access while writing CSV data.

Bugzilla Id: 60818

Modified:
    jmeter/trunk/src/core/org/apache/jmeter/reporters/ResultCollector.java
    jmeter/trunk/xdocs/changes.xml
Comment 2 Sebb 2017-03-04 12:46:02 UTC
I don't think this is the solution.

AFAICT println() is thread-safe, as it uses its own internal lock, at least in the JVM that I am using. So synchronising this won't help.

I would want to see more evidence what the problem is.

Note that the same Writer may be used by SaveService.saveSampleResult and SaveService.saveTestElement (this is called by ResultCollector.recordStats which is probably redundant now that MonitorHealthVisualizer has been dropped)

These are both synchronised, however that won't help if other methods use a different lock.

Methods that write to a Writer from different threads need to use a shared lock.
Since the Writer and subclasses internally use Writer.lock (protected), apps need to do the same to ensure multiple writes are not interleaved.
Comment 3 Felix Schumacher 2017-03-04 14:18:33 UTC
Date: Sat Mar  4 14:15:10 2017
New Revision: 1785488

URL: http://svn.apache.org/viewvc?rev=1785488&view=rev
Log:
Revert r1785464 based on sebb's comment.

JMeterThread does not honor contract of JMeterStopTestNowException

Bugzilla Id: 60812

Modified:
    jmeter/trunk/src/core/org/apache/jmeter/reporters/ResultCollector.java
    jmeter/trunk/xdocs/changes.xml
Comment 4 Philippe Mouawad 2017-03-04 21:39:33 UTC
As per:
http://stackoverflow.com/questions/11072253/is-printwriter-thread-safe

And reading code and javadocs of PrintWriter and Writer, I think Felix code won't hurt as there is indeed no promise in PrintWriter that it will be always thread safe.

Still, I am not sure the issue faced by user is due to that.
@Felix, did you reproduce some corruption yourself ?

Thanks
Regards
Comment 5 Felix Schumacher 2017-03-05 13:06:56 UTC
(In reply to Philippe Mouawad from comment #4)
> As per:
> http://stackoverflow.com/questions/11072253/is-printwriter-thread-safe
> 
> And reading code and javadocs of PrintWriter and Writer, I think Felix code
> won't hurt as there is indeed no promise in PrintWriter that it will be
> always thread safe.

Superfluos synchronisation may hurt, so it was correct to remove my patch.

> 
> Still, I am not sure the issue faced by user is due to that.
> @Felix, did you reproduce some corruption yourself ?

I had seen such a corruption, but I have no way of reproducing it.

> 
> Thanks
> Regards
Comment 6 Philippe Mouawad 2017-03-05 13:10:41 UTC
(In reply to Felix Schumacher from comment #5)
> (In reply to Philippe Mouawad from comment #4)
> > As per:
> > http://stackoverflow.com/questions/11072253/is-printwriter-thread-safe
> > 
> > And reading code and javadocs of PrintWriter and Writer, I think Felix code
> > won't hurt as there is indeed no promise in PrintWriter that it will be
> > always thread safe.
> 
> Superfluos synchronisation may hurt, so it was correct to remove my patch.

That's why I was initially not ok with the patch.
But reading Javadocs, we may be taking a risk by not doing it.
And I don't know how it behaves in other JVMs than Oracle/OpenJDK

> 
> > 
> > Still, I am not sure the issue faced by user is due to that.
> > @Felix, did you reproduce some corruption yourself ?
> 
> I had seen such a corruption, but I have no way of reproducing it.
> 
> > 
> > Thanks
> > Regards
Comment 7 Philippe Mouawad 2017-03-08 20:40:18 UTC

*** This bug has been marked as a duplicate of bug 60830 ***