You are viewing a plain text version of this content. The canonical link for it is here.
Posted to log4net-dev@logging.apache.org by Joe <Jo...@hotmail.com> on 2016/10/21 19:57:00 UTC

Should an Appender be IDisposable?

I think the answer is probably yes: a FileAppender owns an IDisposable Stream, and other appenders probably own IDisposable objects.

I would suggest having AppenderSkeleton implement IDisposable, rather than IAppender.  The Dispose method would call OnClose.

I also notice that AppenderSkeleton has a finalizer, which it shouldn't as it doesn't own any unmanaged resources.

Do you see any problems with making the above changed?

Re: Should an Appender be IDisposable?

Posted by Stefan Bodewig <bo...@apache.org>.
On 2016-10-21, Joe wrote:

> I think the answer is probably yes: a FileAppender owns an IDisposable
> Stream, and other appenders probably own IDisposable objects.

Probably. Likely this has been overlooked when log4net was started.

> I would suggest having AppenderSkeleton implement IDisposable, rather
> than IAppender.  The Dispose method would call OnClose.

Looking at AppenderSkeleton it seems calling Close from Dispose would be
more appropriate.

> I also notice that AppenderSkeleton has a finalizer, which it
> shouldn't as it doesn't own any unmanaged resources.

If we remove this we should check whether any subclasses own unmanaged
resources and add finalizers there. TBH I don't know whether we've got
any Appenders owning unmanaged resources at all. LocalSyslogAppender
looks like a candidate at first glance.

Stefan