RE: AsyncAppenderSkeleton

[Joe <Jo...@hotmail.com>]

I've created https://issues.apache.org/jira/browse/LOG4NET-529 for this


From: Dominik Psenner [mailto:dpsenner@gmail.com]
Sent: 31 October 2016 15:33
To: log4net-dev@logging.apache.org
Subject: Re: AsyncAppenderSkeleton


These few lines alone are very suspicious indeed. Without further investigations I'm unable to give any sensible feedback.
On 2016-10-31 11:54, Joe wrote:
Another point is that for any asynchronous appender implementation, it's a sine qua non that the LoggingEvent class be thread-safe.

At least for the manipulations an appender might do: layout, accessing properties, fixing properties.

I had a quick browse through the source and found what looks to me like a race condition here:

             public object LookupProperty(string key)
             {
                    if (m_data.Properties != null)
                    {
                           return m_data.Properties[key];
                    }
                    if (m_compositeProperties == null)
                    {
                           CreateCompositeProperties();
                    }
                    return m_compositeProperties[key];
             }

             private void CreateCompositeProperties()
             {
                    m_compositeProperties = new CompositeProperties();

                    if (m_eventProperties != null)
                    {
                           m_compositeProperties.Add(m_eventProperties);
                    }
                    ... etc

If two threads call LookupProperty concurrently, one may start executing CreateCompositeProperties and the other might access m_compositeProperties before it's fully created.
If I'm right, the fix is simple - don't set m_compositeProperties until it's completely created.