Is this locking code efficient?

I have a StreamWriter which gets written to.

When the line count gets to 500 it closes it an开发者_JAVA技巧d moves the file.

I have added a timer as well so every 5 seconds it closes it and moves the file.

Obviously if the timer kicks in and closes the StreamWriter and then in MyMethod it tries to write to the StreamWriter it will throw a wobbly. I have therefore added some locks in to try and prevent any issues so if the timer kicks in it closes the StreamWriter, allocates a new file name and then after the lock if MyMethod tries to write to it all should be ok.

Is the below code good enough to handle any issues do you think?

    private readonly object objLock = new object();

    private StartUpMethod()
    {
        if (tmFileWriter == null)
        {
         tmFileWriter = new Timer(5000);
         tmFileWriter.AutoReset = false;
         tmFileWriter.Elapsed += new ElapsedEventHandler(tmFileWriter_Elapsed);
        }
    }

    private void MyMethod()
    {
        lock (objLock)
        {
           if (_tempFilename == "")
           {
              _tempFilename = GenerateFileName();
             _tw = new System.IO.StreamWriter(_tempFilename);
           }
        }

         //Do some processing

         lock (objLock)
         {
            _tw.WriteLine(sql);
            _filelineCount++;
            if (_filelineCount > 500)
            {
               _tw.Close();
               System.IO.File.Move(_tempFilename, _tempFilename.Replace(".tmp", ".sql"));
               _tempFilename = "";
               _filelineCount = 0;
            }
   }

    private void tmFileWriter_Elapsed(object sender, ElapsedEventArgs e)
    {
        tmFileWriter.Stop();

        lock (objLock)
        {
            if (_tw != null)
            {
                _tw.Close();
                 System.IO.File.Move(_tempFilename, _tempFilename.Replace(".tmp", ".sql"));
                 _tempFilename = GenerateFileName();
                 _tw = new StreamWriter(_tempFilename);
             }
        }

        tmFileWriter.Start();
    }
}

That should work well, as it protects the file from concurrent access by multiple threads. The only thing I'd change is to combine the two sections in MyMethod. There's no reason to release the lock after creating the file, just so you can obtain the lock again in order to write. So rather than:

lock
{
  // Create file if necessary
}

lock
{
  // write to the file
}

Just do:

lock
{
  // create file if necessary
  // write to the file
}

The only issue I see is if the timer fires and there are 0 lines entered it could make a unnecessary file, but that could be the behavior you want.

What would happen if the timer fires straight after exiting the second lock in MyMethod.

private void MyMethod() {
  lock{}
  //Do processing
  lock{}
}

Would it try to move a file that has already been moved?

The code looks like it'll work, but if efficiency is an issue, I would've went the route of using buffers and using atomic swaps to change which file to output the buffer to.

Then writing to the buffer wouldn't have any locks involved, only the writing to the file would.