RFR: JDK-6823527: java.util.logging.Handler has thread safety issues

[Jason Mehrens]

> Yes - this is a possibility I envisaged too.
> But would that be better?
> 
> I didn't want to remove any synchronized blocks because I'm
> really not sure of what consequences it might have. 
> getLevel()/setLevel() are already synchronized on Handler.
> It is my belief that most operation already need to call
> getLevel(). So synchronizing on getFilter/setFilter etc.
> should not be such a big issue.
The only reason that I can think of for synchronizing any of the j.u.Handler methods is that the code was created prior to the JMM revision of volatile in JDK 1.5.  The lock policy is required for the 3 abstract methods so that a single LogRecord is published all or nothing.  The level, filter, error manager, encoding could all be declared volatile with no locking.  For the subclasses the locking for publish is too coarse.  The isLoggable method should be called outside the lock. 
> Also - synchronizing on 'this' has the advantage that the lock
> can be shared with subclasses - I mean - that's what a
> subclass would usually expect...
The same was true for COWList (http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=6282140) but, the lock policy changed over time.   This is bug is about thread safety issues so subclass should have little expectations.
 

> But if I hear strong opinions that 'volatile' is definitely better
> for this case I'm prepared to alter my patch.
> I personally tend to use 'volatile' only sparsely - when it's clear
> that it's the better solution. The common case is that Handler.getFoo are a hot methods and Handler.setFoo are cold.  You could always declare the level, filter, error manager, and encoding volatile and have only the setFoo methods synchronize when setting the property. Jason