On Oct 2, 2014, at 7:24 PM, Mark Thomas <ma...@apache.org> wrote:
>> @@ -311,12 +310,8 @@
>> // Override getMessage to avoid creating objects and formatting
>> // dates unless the log message will actually be used.
>> @Override
>> - public String getMessage() {
>> - String msg;
>> - synchronized(format) {
>> - msg = format.format(new Date(_createdTime));
>> - }
>> - return msg;
>> + public synchronized String getMessage() {
>> + return format.format(new Date(_createdTime));
>> }
>> }
>> }
>
> -1. The sync should be as narrow as possible and only on the object that
> needs to be sync'd. Move the sync to the method changes the lock object
> to the instance and will increase the possibility of contention for
> those actions that require the lock to be on the instance.
>
> Mark
Mark, all,
please disregard this last chunk because it is indeed incorrect: I was confused
by the (commented out) @GuardedBy("this") annotation.
In fact my change would not increase contention but would actually decrease
contention (ina bad way) to the point that the thread safe access to the
"format" field would be compromised: different threads would get a lock on
different instances of AbandonedObjectCreatedException and would be able to
access the format object concurrently. My bad.
Jacopo
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
For additional commands, e-mail: dev-h...@commons.apache.org
