On 2 October 2014 18:48, Jacopo Cappellato <jacopo.cappell...@gmail.com> wrote:
>
> 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.
The comment was wrong; I have fixed it.
> 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
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
For additional commands, e-mail: dev-h...@commons.apache.org
