DO NOT REPLY [Bug 23092] - Map.debug/verbosePrint Thread Safety

[bu...@apache.org]

DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG 
RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT
<http://nagoya.apache.org/bugzilla/show_bug.cgi?id=23092>.
ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND 
INSERTED IN THE BUG DATABASE.

http://nagoya.apache.org/bugzilla/show_bug.cgi?id=23092

Map.debug/verbosePrint Thread Safety





------- Additional Comments From apacheBugzilla@AMammenT.cotse.net  2003-09-11 03:36 -------
First of all, after Stephen's original comment, I created a patch and came 
back online in order to create this bug and attach the patch.  I will do so 
anyway, though whether or not this is the appropriate patch has yet to be 
determined.  The patch makes use of the fact that debugPrint and verbosePrint 
both delegate to verbosePrintInternal to pass on an extra variable 
(indentDepth) removing the need for that state to be represented as part of 
the class state.

The old printIndent method has been deprecated, though, as you will see from 
the comments I include in the patch, it doesn't make much sense to do so.

That said, I'd like to address here Janek's comment about preventing 
overlapping invokations from writing to System.out at the same time.  I can 
see why that would be useful, but I think forcing the synchronization on the 
class (particularly a utility class like this) is the worst way to do it.  
This will prevent, as Janek notes, overlapping invokations to debugPrint and 
verbosePrint from interfering with each other.  However....

1) There is no lock on System.out.  In a multi-threaded environment, other 
sources of output outside this class may well be writing to System.out at the 
same time.  Such output may be interlaced with output from these two 
synchronized methods - there is no guard against such behavior.  As a result, 
preventing interlacing of output will most likely require an external guard 
anyway.  

2) The cost of synchronizing these two methods is that none of the otherwise 
thread safe methods in this utility class can be used while any thread is 
using either debugPrint or verbosePrint.  Any thread desiring to use any of 
the other utilities will be blocked until all output is written.  

IMHO, the two reasons above both suggest that the synchronized keyword should 
be removed - any synchronization should be external - perhaps with the 
additional class level documentation that the methods themselves are thread-
safe and do not require explicit synchronization.