RFR: 8014814 (str) StringBuffer "null" is not appended

David Holmes david.holmes at oracle.com
Mon May 20 05:48:41 UTC 2013


The change put through for JDK-8013395 (StringBuffer toString cache) has 
exposed a previously unnoticed bug in the 
StringBuffer.append(CharSequence cs) method. That method claimed to 
achieve synchronization (and then correct toStringCache behaviour) by 
the super.append method calling other StringBuffer methods after 
narrowing of cs to a specific type. But that is incorrect if cs==null as 
in that case the AbstractStringBuilder.appendNull method is called 
directly, with no calls to an overridden StringBuffer method. (I have 
verified that none of the other methods claiming to not need sync suffer 
from a similar flaw - this is an isolated case.)

Consequently we started failing some existing append(null) tests.

The fix is quite simple: append(CharSequence) behaves as for other 
append methods and is declared synchronized and clears the cache 
explicitly. The existing test is extended to check append(null).

webrev:

http://cr.openjdk.java.net/~dholmes/8014814/webrev/

This fix does mean that recursive synchronization will now be used for 
append(CharSequence) but recursive synchronization is very cheap. An 
alternative fix suggested by Alan Bateman in the bug report is to 
override appendNull and add the synchronization there. That requires a 
change to the accessibility of AbstractStringBuilder.appendNull so I 
chose the more constrained fix. Alan's fix will also introduce nested 
synchronization, but only for the append(null) case. As I said I don't 
think performance will be a concern here.

Testing (in progress): JPRT -testset core, SQE test that caught this

Thanks,
David



More information about the core-libs-dev mailing list