On 13/09/2015 23:11, Caldarale, Charles R wrote:
>> From: ma...@apache.org [mailto:ma...@apache.org] 
>> Subject: svn commit: r1702821 - in /tomcat/tc8.0.x/trunk: 
>> java/org/apache/tomcat/util/http/HttpMessages.java webapps/docs/changelog.xml
> 
>> --- tomcat/tc8.0.x/trunk/java/org/apache/tomcat/util/http/HttpMessages.java 
>> (original)
>> +++ tomcat/tc8.0.x/trunk/java/org/apache/tomcat/util/http/HttpMessages.java 
>> Sun Sep 13 20:36:40 2015
>> @@ -69,26 +69,34 @@ public class HttpMessages {
>>          // Does HTTP requires/allow international messages or
>>          // are pre-defined? The user doesn't see them most of the time
>>          switch( status ) {
>> -        case 200:
>> -            if(st_200 == null ) {
>> -                st_200 = sm.getString("sc.200");
>> +        case 200: {
>> +            String s = st_200;
>> +            if(s == null ) {
>> +                st_200 = s = sm.getString("sc.200");
>>              }
>> -            return st_200;
> 
> I'm not convinced this removes the data race, since there's still no 
> guarantee that the storage writes that are part of the object initialization 
> inside sm.getString() will be visible to another thread before the write to 
> st_200. Shipilёv's blog has some interesting studies: 
> http://shipilev.net/blog/2014/safe-public-construction/

OK. Let me have a read of that blog.

At the moment we have two people who claim to be expert in this area
with different opinions on whether or not the above is safe. Personally,
my money is on Chuck being right.

I don't know the JMM well enough to have a definitive opinion at this
point. I suspect that is going to have to change. This is why I *really*
want to see an explanation if why a data race occurs here - with
references to the JMM to back it up - not only to justify the work in
this area but to help those who are new to the JMM to understand exactly
why this is a problem. The more of us that understand this, the more
likely we are as a community to avoid issues like this in the future
and/or to spot them if one of us makes a mistake.

> P.S.  The formatting looks a little odd here, with no space after an "if" nor 
> its opening parenthesis, but a space before the closing one.

I spotted that and meant to fix it but got distracted and forgot. It
looks like I'm going to have to revist this code so I'll get that fixed
as well.

>  Also, the braces on the case statements aren't needed; using them results in 
> having two closing braces in the same column at the end of the switch.

The braces are required because of the repeat declarations of String s.

Mark


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org

Reply via email to