Hi Daniel,
On the surface these changes seem reasonable - the code looks the way it
should have been written in the first place.
The risk with these kinds of changes are always performance and
deadlocks. :) I would not be surprised, given this is logging, that
somewhere there is a path that might lead to a deadlock - but I have no
practical way to ascertain that.
Wait for a true core-libs Reviewer before pushing this one. :)
David
On 29/08/2013 8:42 PM, Daniel Fuchs wrote:
Hi,
This is a fix for an old bug which rightfully remarks that
whereas access to 'level' in handler is synchronized,
access to other mutable fields - like 'filter' etc...
is not - which is inconsistent.
<http://cr.openjdk.java.net/~dfuchs/webrev_6823527/webrev.00/>
I have tried to keep the fix simple - simply adding
synchronized to method declarations when it was clear that
there was an inconsistency.
I also looked at the subclasses of j.u.l.Handler
In some places I resorted to using a synchronized block to avoid
calling overridable / external method from within the lock.
The changed methods are simple accessors, and subclasses of
Handler are already full of synchronized methods (e.g.
methods like publish(...) are already synchronized)
so the risk of fixing these simple accessors should be limited.
best regards,
-- daniel
