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

Sergio Bossa commented on CASSANDRA-11048:
------------------------------------------

[~thobbs],

the fix looks good, but the test doesn't seem correct to me, for the following 
reasons:
* The call to {{fail(exc.getMessage())}} inside the {{Runnable}} doesn't 
actually make the test fail, because it doesn't propagate to the main thread: 
you should either wait on the futures, or "capture" the exception by yourself.
* The call to {{executor.awaitTermination(1, TimeUnit.MINUTES)}} should be 
asserted true, so the test will fail if the other threads hang for any reason.

Other than that, on a more general note, the method might have been tested in 
isolation via an actual unit test that only verifies the quoted string is not 
messed up, without having to create tables and rows, but I see the rest of the 
test class is made like that, so I will not object.

> JSON queries are not thread safe
> --------------------------------
>
>                 Key: CASSANDRA-11048
>                 URL: https://issues.apache.org/jira/browse/CASSANDRA-11048
>             Project: Cassandra
>          Issue Type: Bug
>            Reporter: Sergio Bossa
>            Assignee: Tyler Hobbs
>            Priority: Critical
>              Labels: easyfix, newbie, patch
>         Attachments: 
> 0001-Fix-thread-unsafe-usage-of-JsonStringEncoder-see-CAS.patch
>
>
> {{org.apache.cassandra.cql3.Json}} uses a shared instance of 
> {{JsonStringEncoder}} which is not thread safe (see 1), while 
> {{JsonStringEncoder#getInstance()}} should be used (see 2).
> As a consequence, concurrent {{select JSON}} queries often produce wrong 
> (sometimes unreadable) results.
> 1. 
> http://grepcode.com/file/repo1.maven.org/maven2/org.codehaus.jackson/jackson-core-asl/1.9.2/org/codehaus/jackson/io/JsonStringEncoder.java
> 2. 
> http://grepcode.com/file/repo1.maven.org/maven2/org.codehaus.jackson/jackson-core-asl/1.9.2/org/codehaus/jackson/io/JsonStringEncoder.java#JsonStringEncoder.getInstance%28%29



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Reply via email to