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: [EMAIL PROTECTED] [mailto:[EMAIL PROTECTED] > Sent: 11 March 2005 18:38 > To: [EMAIL PROTECTED] > 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(); > > > > >
