tags:

views:

361

answers:

5

I am doing an archive service for logs. Since some files are open and in use I can't archive them until they are not in use anymore. The only way I can figure out how to see if the file is open by another process is the code below.

I hate the code there must be a better way. Is there a better way?

try
{
    using (FileStream stream = File.Open("SomeFile.txt", FileMode.Open, FileAccess.ReadWrite))
    {
        // Do nothing just seeing if I could open it.
        return true;
    }
}
catch (IOException)
{
    // Exception so File is in use and can't open
    return false;
}

EDIT: Please note, that I do not want to catch the exception. I would rather avoid the exception entirely. I would rather have some magic function for example IsFileInUser(string fileName) that would return a boolean WITHOUT catching exceptions underneath.

A way I have see is using OpenFile of PInvoke.

http://www.pinvoke.net/default.aspx/kernel32.OpenFile

+1  A: 

This question has appeared on SO before. Search for file locked / file locking. To summarize the answers: no, there's no better reliable way.

You could, in theory, use Windows API to find open IO handles to the file, but even if you see that file is not locked, and proceed with your logic, the same file can get locked again just before you try to access it:

if FileIsNotLocked(file) {
   ...                         <=== file can get re-locked here!
   do something
}
zvolkov
+3  A: 

The only thing I would improve on in your code example is catching the few specific exceptions that would be thrown by File.Open. You'll need to catch IOException and UnauthorizedAccessException at a minimum.

By putting in catch, you catch any exception that might be thrown, which is bad practice.

jasonh
I understand this, however for brevity I just put in catch.
David Basarab
OK, I just wanted to be sure for the sake of posterity. If someone else comes along and reads it, they will know not to do a catch-all type `catch` statement.
jasonh
+1  A: 

To avoid the problem mentioned by zvolkov, if you don't mind locking out other applications while you do your archiving, then you could try opening the file with a lock which denies writers but not readers. Then go ahead and archive the file. Of course this will lock out any application which tries to open the file for update - but given that you want to archive the file at some point, there's no really good way out of the dilemma.

using (FileStream stream = new FileStream(logFilePath, FileMode.Open, FileAccess.Read, FileShare.Read))
{
   // Archive your log.
}
Vinay Sajip
A: 

I think you just have to catch the exception. I would just combine the reading and testing into one, maybe like this.

bool success = false;

while( !success )
{
  try
  {
    //TODO: Try to open the file and write to it
    success = true;
  }
  catch( IOException ex )
  {
     // File in use, wait a little bit and try again.
     Thread.Sleep(xxx);
  }
}
Greg
+1  A: 

Here is what I came up with.

First here is the bad way of doing it by exception.

public static bool IsFileInUseBadMethod(string filePath)
{
    if (!File.Exists(filePath))
    {
        return false;
    }

    try
    {
        using (StreamReader reader = new StreamReader(File.Open(filePath, FileMode.Open, FileAccess.ReadWrite)))
        {
            return false;
        }
    }
    catch (IOException)
    {
        return true;
    }
}

On my machine this method took about 10 million ticks when the file was in use.

The other way is using the kernel32.dll

// Need to declare the function from the kernel32
[DllImport("kernel32.dll", SetLastError = true, CharSet = CharSet.Unicode)]
private static extern Microsoft.Win32.SafeHandles.SafeFileHandle CreateFile(string lpFileName, System.UInt32 dwDesiredAccess, System.UInt32 dwShareMode, IntPtr pSecurityAttributes, System.UInt32 dwCreationDisposition, System.UInt32 dwFlagsAndAttributes, IntPtr hTemplateFile);

private static readonly uint GENERIC_WRITE = 0x40000000;
private static readonly uint OPEN_EXISTING = 3;

Here is the function

public static bool IsFileInUse(string filePath)
{
    if (!File.Exists(filePath))
    {
        return false;
    }

    SafeHandle handleValue = null;

    try
    {
        handleValue = FileHelper.CreateFile(filePath, FileHelper.GENERIC_WRITE, 0, IntPtr.Zero, FileHelper.OPEN_EXISTING, 0, IntPtr.Zero);

        bool inUse = handleValue.IsInvalid;

        return inUse;
    }
    finally
    {
        if (handleValue != null)
        {
            handleValue.Close();

            handleValue.Dispose();

            handleValue = null;
        }
    }
}

This ran in about 1.8 million ticks. Over an 8 million ticks difference from the exception method.

However when file it not in use meaning no exception is thrown the speed of the "BAD" method is 1.3 million and the kernel32.dll is 2 million. About an 800k ticks better. So using the Exception method would be better if I knew the file would be not be in use about 1/10th of the time. However the one time it is, it is an extremely expensive ticks operation compared to the kernel32.dll.

David Basarab