logging-log4j-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Remko Popma <remko.po...@gmail.com>
Subject Re: LoggerContext locking
Date Tue, 07 Apr 2015 09:15:58 GMT
For testing, just a thought: what about creating a (test) plugin that
during its initialization triggers a reconfiguration in another thread?

The other thread should not be able to complete reconfiguration until the
first configuration completes. Not sure if this can be turned into a useful
test...

On Tuesday, April 7, 2015, Ralph Goers <ralph.goers@dslextreme.com> wrote:

> I agree with all of this. However, I believe the reconfigure method does
> not need to be synchronized. I think it is OK to construct a new
> Configuration while not synchronized. What needs to be locked is the
> setConfigurationMethod, which can I believe can be locked using the
> configLock, rather than synchronizing the method.
>
> The problem with this is I am not sure I know of a good way to verify all
> of this. The error that is occurring happens because the FlumeAppender is
> trying to shutdown its writer threads during a reconfigure and one of those
> threads is trying to flush data which apparently is causing  a new Logger
> to be obtained in the process of doing that.
>
> I can try to create a test case that emulates this but it may be a bit
> tricky.
>
> Ralph
>
> > On Apr 6, 2015, at 6:55 PM, Remko Popma <remko.popma@gmail.com
> <javascript:;>> wrote:
> >
> > Looking at the patch I submitted for LOG4J2-207, this is where the
> synchronized methods getConfigLocation and setConfigLocation were added.
> The patch also made the configLocation field mutable (it was final before).
> >
> > I think my reasoning for making them synchronized was that setting the
> configLocation field and the subsequent call to reconfigure() should be
> atomic.
> >
> > I agree that there may be better ways of doing this. One idea:
> > 1. Make configLocation volatile as Ralph suggested
> > 2. Remove synchronized keyword from getConfigLocation and
> setConfigLocation
> > 3. Add a new private method reconfigure(URI configLocation). This does
> all the work of the current reconfigure() method, but uses the specified
> configLocation method parameter instead of the field.
> > 4. Modify the public synchronized method reconfigure() to delegate to
> the private method with the configLocation field value. (So this method
> becomes a one-liner.)
> >
> > This is a cleaner way to preserve the atomicity of the setConfiguration
> method without any additional locks.
> >
> > Thoughts?
> >
> > Sent from my iPhone
> >
> >> On 2015/04/07, at 1:02, Ralph Goers <ralph.goers@dslextreme.com
> <javascript:;>> wrote:
> >>
> >> I’ve been looking at the deadlock that is documented in LOG4J-982.  It
> seems to me that the problem is that get/setConfigLocation and reconfigure
> are locking the whole LoggerContext.  First, the history shows that I added
> get & setConfigLocation as part of LOG4J2-207. I think Remko actually wrote
> the code.  I am not sure why these methods need to be synchronized.
> Wouldn’t it be enough for configLocation to be volatile?
> >>
> >> Second, I don’t think locking the LoggerContext for the entirety a
> reconfigure is a good idea. reconfigure calls setConfiguration, which is
> also synchronized. It seems to me that while setConfiguration does need to
> be locked, it should be using a lock that specifically only prevents
> setConfiguration from being executed simultaneously from more than one
> thread.
> >>
> >> Thoughts?
> >>
> >> Ralph
> >> ---------------------------------------------------------------------
> >> To unsubscribe, e-mail: log4j-dev-unsubscribe@logging.apache.org
> <javascript:;>
> >> For additional commands, e-mail: log4j-dev-help@logging.apache.org
> <javascript:;>
> >>
> >
> > ---------------------------------------------------------------------
> > To unsubscribe, e-mail: log4j-dev-unsubscribe@logging.apache.org
> <javascript:;>
> > For additional commands, e-mail: log4j-dev-help@logging.apache.org
> <javascript:;>
> >
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: log4j-dev-unsubscribe@logging.apache.org
> <javascript:;>
> For additional commands, e-mail: log4j-dev-help@logging.apache.org
> <javascript:;>
>
>

Mime
View raw message