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 Nicko Cadell <ni...@neoworks.com> on 2005/03/12 00:03:00 UTC
RE: FileAppender Locking
Niall,
It looks like the LockingStream AcquireLock and ReleaseLock calls should
not be nested, e.g. it would be bad if AcquireLock was called twice
without an intermediate ReleaseLock. The implementation in LockingStream
does not check for this. I think that it would be sufficient to add
error checking to LockingStream so that AcquireLock errors if the
m_realStream is not null and ReleaseLock errors if m_realStream is null.
The other alternative would be to add reference counting to
LockingStream and only null the m_realStream when the count reaches
zero.
Would there be any advantage to making LockingStream.AcquireLock return
true if the lock was acquired, and false otherwise? This would mean that
the FileAppender could test for this and not bother actually trying to
write if the lock has not been obtained.
The locking model will need access to a SecurityContext which it should
use to impersonate around calls to the file system. It looks like the
FileAppender will impersonate for the LockingModel.OpenFile call, but
not for the AcquireLock calls. Of course it is down to the lock
implementation to know when to impersonate.
It may be easiest to give the LockingModelBase a reference back to the
FileAppender. It can then use this to retrieve the ErrorHandler as well
as the SecurityContext. Then it may be best to also delegate the
impersonation for OpenFile to the LockingModel rather than doing that in
the FileAppender.
Cheers,
Nicko
> -----Original Message-----
> From: niall@apache.org [mailto:niall@apache.org]
> Sent: 11 March 2005 18:38
> To: logging-log4net-cvs@apache.org
> Subject: cvs commit: logging-log4net/src/Appender FileAppender.cs
>
> niall 2005/03/11 10:38:07
>
> Modified: src/Appender FileAppender.cs
> Log:
> Added documentation for the locking models in FileAppender.
>
> Revision Changes Path
> 1.15 +33 -1 logging-log4net/src/Appender/FileAppender.cs
>
> Index: FileAppender.cs
> ===================================================================
> RCS file: /home/cvs/logging-log4net/src/Appender/FileAppender.cs,v
> retrieving revision 1.14
> retrieving revision 1.15
> diff -u -r1.14 -r1.15
> --- FileAppender.cs 11 Mar 2005 18:21:56 -0000 1.14
> +++ FileAppender.cs 11 Mar 2005 18:38:07 -0000 1.15
> @@ -218,12 +218,18 @@
> }
>
>
>
> /// <summary>
>
> - /// Open te file once for writing and hold it
> open until CloseFile is called. Maintains an exclusive lock
> on the file during this time.
>
> + /// Open the file once for writing and hold it
> open until CloseFile is called. Maintains an exclusive lock
> on the file during this time.
>
> /// </summary>
>
> public class ExclusiveLock : LockingModelBase
>
> {
>
> private Stream m_stream=null;
>
>
>
> + /// <summary>
>
> + /// Open the file specified and prepare
> for logging.
>
> + /// </summary>
>
> + /// <param name="filename">The filename
> to use</param>
>
> + /// <param name="append">Whether to
> append to the file, or overwrite</param>
>
> + /// <param name="encoding">The encoding
> to use</param>
>
> public override void OpenFile(string
> filename, bool append,Encoding encoding)
>
> {
>
> try
>
> @@ -247,16 +253,26 @@
> }
>
> }
>
>
>
> + /// <summary>
>
> + /// Close the file.
>
> + /// </summary>
>
> public override void CloseFile()
>
> {
>
> m_stream.Close();
>
> }
>
>
>
> + /// <summary>
>
> + /// Does nothing. The lock is already taken
>
> + /// </summary>
>
> + /// <returns>A stream that is ready to
> be written to.</returns>
>
> public override Stream AquireLock()
>
> {
>
> return m_stream;
>
> }
>
>
>
> + /// <summary>
>
> + /// Does nothing. The lock will be
> released when the file is closed.
>
> + /// </summary>
>
> public override void ReleaseLock()
>
> {
>
> //NOP
>
> @@ -274,17 +290,30 @@
> private bool m_append;
>
> private Stream m_stream=null;
>
>
>
> + /// <summary>
>
> + /// Prepares to open the file when the
> first message is logged.
>
> + /// </summary>
>
> + /// <param name="filename">The filename
> to use</param>
>
> + /// <param name="append">Whether to
> append to the file, or overwrite</param>
>
> + /// <param name="encoding">The encoding
> to use</param>
>
> public override void OpenFile(string
> filename, bool append, Encoding encoding)
>
> {
>
> m_filename=filename;
>
> m_append=append;
>
> }
>
>
>
> + /// <summary>
>
> + /// Ensures the file is closed.
>
> + /// </summary>
>
> public override void CloseFile()
>
> {
>
> //NOP
>
> }
>
>
>
> + /// <summary>
>
> + /// Aquire the lock on the file in
> preparation for writing to it. Return a stream pointing to the file.
>
> + /// </summary>
>
> + /// <returns>A stream that is ready to
> be written to.</returns>
>
> public override Stream AquireLock()
>
> {
>
> if (m_stream==null)
>
> @@ -313,6 +342,9 @@
> return m_stream;
>
> }
>
>
>
> + /// <summary>
>
> + /// Release the lock on the file. No
> further writes will be made to the stream until AquireLock is
> called again.
>
> + /// </summary>
>
> public override void ReleaseLock()
>
> {
>
> m_stream.Close();
>
>
>
>
>
RE: FileAppender Locking
Posted by Niall Daley <ni...@apache.org>.
Nicko,
I've made the changes you're suggesting, plus reimplemented the
read methods on the LockingStream. We don't need them now, but I can
envisage situations where they might be useful.
Niall
On Fri, 11 Mar 2005, Nicko Cadell wrote:
> Niall,
>
> It looks like the LockingStream AcquireLock and ReleaseLock calls should
> not be nested, e.g. it would be bad if AcquireLock was called twice
> without an intermediate ReleaseLock. The implementation in LockingStream
> does not check for this. I think that it would be sufficient to add
> error checking to LockingStream so that AcquireLock errors if the
> m_realStream is not null and ReleaseLock errors if m_realStream is null.
> The other alternative would be to add reference counting to
> LockingStream and only null the m_realStream when the count reaches
> zero.
>
> Would there be any advantage to making LockingStream.AcquireLock return
> true if the lock was acquired, and false otherwise? This would mean that
> the FileAppender could test for this and not bother actually trying to
> write if the lock has not been obtained.
>
> The locking model will need access to a SecurityContext which it should
> use to impersonate around calls to the file system. It looks like the
> FileAppender will impersonate for the LockingModel.OpenFile call, but
> not for the AcquireLock calls. Of course it is down to the lock
> implementation to know when to impersonate.
> It may be easiest to give the LockingModelBase a reference back to the
> FileAppender. It can then use this to retrieve the ErrorHandler as well
> as the SecurityContext. Then it may be best to also delegate the
> impersonation for OpenFile to the LockingModel rather than doing that in
> the FileAppender.
>
> Cheers,
>
> Nicko
>
> > -----Original Message-----
> > From: niall@apache.org [mailto:niall@apache.org]
> > Sent: 11 March 2005 18:38
> > To: logging-log4net-cvs@apache.org
> > Subject: cvs commit: logging-log4net/src/Appender FileAppender.cs
> >
> > niall 2005/03/11 10:38:07
> >
> > Modified: src/Appender FileAppender.cs
> > Log:
> > Added documentation for the locking models in FileAppender.
> >
> > Revision Changes Path
> > 1.15 +33 -1 logging-log4net/src/Appender/FileAppender.cs
> >
> > Index: FileAppender.cs
> > ===================================================================
> > RCS file: /home/cvs/logging-log4net/src/Appender/FileAppender.cs,v
> > retrieving revision 1.14
> > retrieving revision 1.15
> > diff -u -r1.14 -r1.15
> > --- FileAppender.cs 11 Mar 2005 18:21:56 -0000 1.14
> > +++ FileAppender.cs 11 Mar 2005 18:38:07 -0000 1.15
> > @@ -218,12 +218,18 @@
> > }
> >
> >
> >
> > /// <summary>
> >
> > - /// Open te file once for writing and hold it
> > open until CloseFile is called. Maintains an exclusive lock
> > on the file during this time.
> >
> > + /// Open the file once for writing and hold it
> > open until CloseFile is called. Maintains an exclusive lock
> > on the file during this time.
> >
> > /// </summary>
> >
> > public class ExclusiveLock : LockingModelBase
> >
> > {
> >
> > private Stream m_stream=null;
> >
> >
> >
> > + /// <summary>
> >
> > + /// Open the file specified and prepare
> > for logging.
> >
> > + /// </summary>
> >
> > + /// <param name="filename">The filename
> > to use</param>
> >
> > + /// <param name="append">Whether to
> > append to the file, or overwrite</param>
> >
> > + /// <param name="encoding">The encoding
> > to use</param>
> >
> > public override void OpenFile(string
> > filename, bool append,Encoding encoding)
> >
> > {
> >
> > try
> >
> > @@ -247,16 +253,26 @@
> > }
> >
> > }
> >
> >
> >
> > + /// <summary>
> >
> > + /// Close the file.
> >
> > + /// </summary>
> >
> > public override void CloseFile()
> >
> > {
> >
> > m_stream.Close();
> >
> > }
> >
> >
> >
> > + /// <summary>
> >
> > + /// Does nothing. The lock is already taken
> >
> > + /// </summary>
> >
> > + /// <returns>A stream that is ready to
> > be written to.</returns>
> >
> > public override Stream AquireLock()
> >
> > {
> >
> > return m_stream;
> >
> > }
> >
> >
> >
> > + /// <summary>
> >
> > + /// Does nothing. The lock will be
> > released when the file is closed.
> >
> > + /// </summary>
> >
> > public override void ReleaseLock()
> >
> > {
> >
> > //NOP
> >
> > @@ -274,17 +290,30 @@
> > private bool m_append;
> >
> > private Stream m_stream=null;
> >
> >
> >
> > + /// <summary>
> >
> > + /// Prepares to open the file when the
> > first message is logged.
> >
> > + /// </summary>
> >
> > + /// <param name="filename">The filename
> > to use</param>
> >
> > + /// <param name="append">Whether to
> > append to the file, or overwrite</param>
> >
> > + /// <param name="encoding">The encoding
> > to use</param>
> >
> > public override void OpenFile(string
> > filename, bool append, Encoding encoding)
> >
> > {
> >
> > m_filename=filename;
> >
> > m_append=append;
> >
> > }
> >
> >
> >
> > + /// <summary>
> >
> > + /// Ensures the file is closed.
> >
> > + /// </summary>
> >
> > public override void CloseFile()
> >
> > {
> >
> > //NOP
> >
> > }
> >
> >
> >
> > + /// <summary>
> >
> > + /// Aquire the lock on the file in
> > preparation for writing to it. Return a stream pointing to the file.
> >
> > + /// </summary>
> >
> > + /// <returns>A stream that is ready to
> > be written to.</returns>
> >
> > public override Stream AquireLock()
> >
> > {
> >
> > if (m_stream==null)
> >
> > @@ -313,6 +342,9 @@
> > return m_stream;
> >
> > }
> >
> >
> >
> > + /// <summary>
> >
> > + /// Release the lock on the file. No
> > further writes will be made to the stream until AquireLock is
> > called again.
> >
> > + /// </summary>
> >
> > public override void ReleaseLock()
> >
> > {
> >
> > m_stream.Close();
> >
> >
> >
> >
> >
>
--
Niall Daley
Log4net Dev