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: [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();
> >
> >
> >
> >
> >
>
--
Niall Daley
Log4net Dev
