commons-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Sebb (Commented) (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (SANDBOX-408) CSVFormat describes itself as immutable, but it is not - in particular it is not thread-safe
Date Tue, 06 Mar 2012 22:33:58 GMT

    [ https://issues.apache.org/jira/browse/SANDBOX-408?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13223744#comment-13223744
] 

Sebb commented on SANDBOX-408:
------------------------------

The Java Memory Model allows threads to cache copies of variables. This is done to improve
performance.

If thread A writes to a field, that write may be cached.
Even if the cache is written to main memory, thread B may already have cached the value so
may not see the update.

If the reader and writer threads use the same lock - or the field is volatile - then the JVM
guarantees that the data will be published correctly. Otherwise, in general the reader may
never see what the writer thread wrote, or may see writes out of order. [There are some further
rules about final variables.]

Unfortunately this is quite difficult to test, as whether and when caching occurs depends
on lots of factors.

So yes, adding volatile would fix this particular problem.
Or one could use final variables for all fields and use a constructor instead of clone.
That would be rather better.

But given that the rest of CSV is not thread-safe, I wonder whether it's necessary to make
the format class thread-safe.
                
> CSVFormat describes itself as immutable, but it is not - in particular it is not thread-safe
> --------------------------------------------------------------------------------------------
>
>                 Key: SANDBOX-408
>                 URL: https://issues.apache.org/jira/browse/SANDBOX-408
>             Project: Commons Sandbox
>          Issue Type: Bug
>          Components: CSV
>            Reporter: Sebb
>
> CSVFormat describes itself as immutable, but it is not @Immutable - the class fields
are all mutable.
> The methods that change the fields do so by creating a clone, and returning the changed
clone.
> So in a sense the class is immutable.
> However, the normal expectation is that @Immutable classes are @ThreadSafe.
> CSVFormat is not thread-safe, because the fields are not volatile, and the fields are
not written & read using a common lock.
> The comment needs to be clarified or removed.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

Mime
View raw message