114

Basically, I would like to check if I have rights to open the file before I actually try to open it; I do not want to use a try/catch for this check unless I have to. Is there a file access property I can check before hand?

John Saunders
  • 160,644
  • 26
  • 247
  • 397
Horas
  • 1,781
  • 3
  • 16
  • 11

6 Answers6

172

I have done this countless times in the past, and nearly every time I was wrong to even make the attempt.

File permissions (even file existence) are volatile — they can change at any time. Thanks to Murphy's Law this especially includes the brief period between when you check the file and when you try to open it. There are other reasons this can fail, too, such as locking or network availability and path resolution. An errant result is even more likely if you're in an area where you know you need to check first. Yet strangely enough it will never happen in your testing or development environments, which tend to be fairly static. This makes the problem difficult to track down later and makes it easy for this kind of bug to make it into production.

What this means is you must still be ready to handle the exception if file permissions or existence are bad, in spite of your check. Exception handling code is required, whether or not you check the file in advance, and good exception handler can provide all of the functionality of existence or permissions checks.

But isn't exception handling slow? I'm glad you asked. Yes. Yes it is. In fact, unwinding the stack to handle an exception is up there with the slowest stuff you can do inside a single computer. However, it's important to remember that disk I/O is even slower — a lot slower — and calling the .Exists() function or checking permissions will always force an additional I/O operation on the file system.

Therefore, we see an initial check before trying to open the file is both redundant and wasteful. There is no additional benefit over exception handling. It will actually hurt, not help, your performance. It adds cost in terms of more code that must be maintained. Finally, it can introduce subtle bugs. There is just no upside at all to doing the initial check.

Instead, the correct thing here is immediately trying to open the file, with no initial check, and putting your effort into a good exception handler if it fails. The same is true whether you're checking permissions, locking, or even just whether or not the file exists.

In summary: the choice is paying the (large) extra cost for a file check every time with more code, or paying the smaller-but-still-bad cost for exception handling only some of the time and with less code.

Joel Coehoorn
  • 399,467
  • 113
  • 570
  • 794
  • 6
    Exactly. This is a classic example of a race condition. – Powerlord Nov 05 '08 at 17:41
  • It would be great if there were a "File System Mutex". But without the existence of that, you are dead right. – Jason Jackson Nov 05 '08 at 18:48
  • Isnt the question about file opening rights/permissions? So first you check if you have permission on this file, and then only if you have permission, you try if it's available? – korro Nov 05 '08 at 19:45
  • 3
    korro: you have to be able to handle bad permissions on failure anyway, and that makes the initial check redundant and wasteful. – Joel Coehoorn Nov 06 '08 at 16:46
  • @chrissie: linux has the same issue. It's a file system thing, and ext2/3/reiser/etc are not immune. – Joel Coehoorn Nov 06 '08 at 16:47
  • Looks like a good explanation.Althoug i will check once before affirming – Ravisha Feb 10 '10 at 07:16
  • 2
    An initial check can help to handle common specific errors gracefully - looking ahead is often easier than matching particular exception attributes to specific causes. The try/catch still remains obligatory. – peterchen Mar 18 '10 at 12:42
  • 7
    This answer does not answer the question "how to check whether I have rights to open the file" at some instance before trying to open it. The case may very well be, that if permission is not allowed at that instance, the software will not attempt to read the file, even if permission may very well be granted just after permissions were checked. – Triynko Nov 29 '11 at 05:41
  • If you're strictly trying to open a file... then sure, committing to open a file handle with permissions as they are at that instance, as an atomic file system operation, is the only way to reliably determine whether you had access. On the other hand, there should be a way to test whether your user account SHOULD have read access to a file at the time you're checking for access. – Triynko Nov 29 '11 at 05:42
  • 5
    It doesn't matter whether permissions are volatile, when you only care what they are at that instant. Failure should always be handled, but if you check for a read permission and it's not there, then you may want to skip reading the file, even if it's possible that a second later you might have access. You have to draw the line somewhere. – Triynko Nov 29 '11 at 05:43
  • Finally, obtaining effective permissions from the ACL is demonstrated here in a few lines of purely managed code: http://stackoverflow.com/a/1281638/88409 It's not that hard. – Triynko Nov 29 '11 at 05:57
  • 2
    Good info, however, I definitely disagree that an extra check is wasteful. That's like saying any check of an expected failure is wasteful, instead let it fail and recover. In C# (as with many other languages that support exceptions), there is a popular paradigm that exceptions are reserved for unrecoverable failures. Often exception code is exceptionally slower than pre-exception tests. – Kit10 Jul 27 '12 at 16:34
  • 1
    @Copperpot It is true that exception handling code is often significantly slower. However, what we save here is an extra trip out to disk. Trips to disk are orders of magnitude slower even than extra exception handling. If you don't like seeing the try/catch in your control flow, well, you could try to hide it away in a method, but part of my point is that you still have to write that code somewhere, because the .Exists() call isn't reliable enough to do without it. – Joel Coehoorn Jul 27 '12 at 19:05
  • 1
    Relaying on exceptions for a false condition is very costly (Stacktrace needs much time to be generated). You are much faster if you can discard a file operation when you check weather you can access it or not, before. – habakuk Aug 03 '12 at 14:44
  • @habakuk that would be nice if you were actually checking before you accessed the file, but that's not the way it works. Calling File.Exists() does require a trip out to disk. As slow as exceptions are, the file io is still **way** worse. – Joel Coehoorn Aug 03 '12 at 14:48
  • Exceptions don't work if you're using some crap closed source library that doesn't throw documented or meaningful exceptions. – Andrew Wagner Jan 21 '16 at 12:52
  • @AndrewWagner But we're not talking about a random library. We're talking mainly about System.IO – Joel Coehoorn Jan 21 '16 at 14:27
  • FWIW: Stack-trace *captures* are relatively cheap - it's the later access to them is what is "most expensive". Where "expensive" is some inconsequential time - ie. *still measured in usecs* - in any reasonable usage this is .. not interesting to fret over. See http://mattwarren.org/2016/12/20/Why-Exceptions-should-be-Exceptional/, eg. So much time worried about a few usec is probably not super productive - one can easily write an own 'TryOpen' and have it *not* dominate program time.. – user2864740 Aug 07 '18 at 03:38
  • @JoelCoehoorn - I'm surprised by your comment that calling File.Exists first requires an extra trip out to disk. No, File.Exists, followed shortly by an attempt to open, will only require a single read of directory info into cache. AFAIK, its not a significant loss of time vs reading w/o the exists check. I personally do File.Exists first (sometimes), because it handles certain conditions (e.g. bad file path) without throwing an exception. – ToolmakerSteve Apr 25 '19 at 20:44
  • I am just now writing an application that needs access to several directories on different servers at different times. In this scenario it totally makes sense to check at startup if all permissions are ok. I will still have to cope with volatile connections, but testing a whole cycle just to realize your user doesn't have the right permission for the final task is not fun. –  Jul 25 '21 at 21:24
26

Quick tip for anyone else coming here with a similar problem:

Watch out for web synchronization apps such as DropBox. I just spent 2 hours thinking the "using" statement (Dispose pattern) is broken in .NET.

I eventually realised that Dropbox is continually reading and writing files in the background, in order to sync them.

Guess where my Visual Studio Projects folder is located? Inside the "My Dropbox" folder of course.

Therefore as I ran my application in Debug mode, the files it was reading and writing were also continually being accessed by DropBox to be synched with the DropBox server. This caused the locking/access conflicts.

So at least I now know that I need to a more robust File Open function (ie TryOpen() that will make multiple attempts). I am surprised it's not already a built-in part of the framework.

[Update]

Here's my helper function:

/// <summary>
/// Tries to open a file, with a user defined number of attempt and Sleep delay between attempts.
/// </summary>
/// <param name="filePath">The full file path to be opened</param>
/// <param name="fileMode">Required file mode enum value(see MSDN documentation)</param>
/// <param name="fileAccess">Required file access enum value(see MSDN documentation)</param>
/// <param name="fileShare">Required file share enum value(see MSDN documentation)</param>
/// <param name="maximumAttempts">The total number of attempts to make (multiply by attemptWaitMS for the maximum time the function with Try opening the file)</param>
/// <param name="attemptWaitMS">The delay in Milliseconds between each attempt.</param>
/// <returns>A valid FileStream object for the opened file, or null if the File could not be opened after the required attempts</returns>
public FileStream TryOpen(string filePath, FileMode fileMode, FileAccess fileAccess,FileShare fileShare,int maximumAttempts,int attemptWaitMS)
{
    FileStream fs = null;
    int attempts = 0;

    // Loop allow multiple attempts
    while (true)
    {
        try
        {
            fs = File.Open(filePath, fileMode, fileAccess, fileShare);

            //If we get here, the File.Open succeeded, so break out of the loop and return the FileStream
            break;
        }
        catch (IOException ioEx)
        {
            // IOExcception is thrown if the file is in use by another process.

            // Check the numbere of attempts to ensure no infinite loop
            attempts++;
            if (attempts > maximumAttempts)
            {
                // Too many attempts,cannot Open File, break and return null 
                fs = null;
                break;
            }
            else
            {
                // Sleep before making another attempt
                Thread.Sleep(attemptWaitMS);

            }

        }

    }
    // Reutn the filestream, may be valid or null
    return fs;
}
Ash
  • 60,973
  • 31
  • 151
  • 169
  • 3
    @Ash I think u didn read question properly HE wants to avoid try catch. – Ravisha Feb 10 '10 at 07:15
  • 11
    @Ravisha, did you even read Joel's top voted answer? As Joel says, **"What you do instead is just try to open the file and handle the exception if it fails"**. Please don't downvote just because you don't like the fact that something cannot be avoided. – Ash Feb 10 '10 at 07:40
  • Thanks for the code! One thing, it might be better to use using e.g. see [Tazeem answer here](http://social.msdn.microsoft.com/Forums/en/netfxbcl/thread/e99a7cea-43d3-49b1-82bc-5669e0b9d052) – Cel Oct 24 '11 at 13:00
  • Given you return the filestream then the `using` would have to be used by the caller though ... – Cel Oct 24 '11 at 14:00
  • @Cel - `using` won't work here. At the end of the using block, `fs` will be forced closed. You will give the caller a CLOSED (so useless) filestream! – ToolmakerSteve Apr 25 '19 at 19:28
4

First, what Joel Coehoorn said.

Also: you should examine the assumptions that underly your desire to avoid using try/catch unless you have to. The typical reason for avoiding logic that depends on exceptions (creating Exception objects performs poorly) probably isn't relevant to code that's opening a file.

I suppose that if you're writing a method that populates a List<FileStream> by opening every file in a directory subtree and you expected large numbers of them to be inaccessible you might want to check file permissions before trying to open a file so that you didn't get too many exceptions. But you'd still handle the exception. Also, there's probably something terribly wrong with your program's design if you're writing a method that does this.

Robert Rossney
  • 94,622
  • 24
  • 146
  • 218
3

Here is the solution you are looking for

var fileIOPermission = new FileIOPermission(FileIOPermissionAccess.Read,
                                            System.Security.AccessControl.AccessControlActions.View,
                                            MyPath);

if (fileIOPermission.AllFiles == FileIOPermissionAccess.Read)
{
    // Do your thing here...
}

this creates a new permission of read based on view for path of all files then checks if it's equal to file access read.

Brandon Minnick
  • 13,342
  • 15
  • 65
  • 123
dj shahar
  • 47
  • 1
  • Not working for any of the file on which I have permissions – Manish Dubey Jan 14 '21 at 07:33
  • Microsoft has **red box warnings** on the [API browser page for FileIOPermissionAccess](https://learn.microsoft.com/en-us/dotnet/api/system.security.permissions.fileiopermission?view=windowsdesktop-7.0) stating that: **"Code Access Security (CAS) has been deprecated across all versions of .NET Framework and .NET. Recent versions of .NET do not honor CAS annotations and produce errors if CAS-related APIs are used. Developers should seek alternative means of accomplishing security tasks."** – wopr_xl Jan 03 '23 at 22:14
-2
public static bool IsFileLocked(string filename)
{
    try
    {
        using var fs = File.Open(filename, FileMode.OpenOrCreate, FileAccess.ReadWrite, FileShare.None);
    }
    catch (IOException)
    {
        return true;
    }
    return false;
}
Tedd Hansen
  • 12,074
  • 14
  • 61
  • 97
Omid Matouri
  • 157
  • 5
  • 1
    For anyone coming here for a quick copy-paste solution, this might be it. FileIOPermission solution in other answer is deprecated in .Net Core. Do read the answer about race condition though. For my current use case this works perfectly. I just need to wait for/detect new file, then detect when write lock is released so I can start processing it. No race condition issue. – Tedd Hansen Jul 28 '21 at 20:49
  • This is functionally the same as [this answer from 2009](https://stackoverflow.com/a/806994/1364007) which also uses IOException. – Wai Ha Lee Jul 29 '21 at 07:07
-4
public static FileStream GetFileStream(String filePath, FileMode fileMode, FileAccess fileAccess, FileShare fileShare, ref int attempts, int attemptWaitInMilliseconds)
{            
    try
    {
         return File.Open(filePath, fileMode, fileAccess, fileShare);
    }
    catch (UnauthorizedAccessException unauthorizedAccessException)
    {
        if (attempts <= 0)
        {
            throw unauthorizedAccessException;
        }
        else
        {
            Thread.Sleep(attemptWaitInMilliseconds);
            attempts--;
            return GetFileStream(filePath, fileMode, fileAccess, fileShare, ref attempts, attemptWaitInMilliseconds);
        }
    }
}
Rudzitis
  • 1
  • 1
  • 9
    -1: use "throw;" not "throw unauthorizedAccessException;". You're losing your stack trace. – John Saunders Mar 18 '10 at 12:54
  • Why is `attempts` passed by ref? That makes no sense. Neither does testing for `<=` instead of just `==`. – Konrad Rudolph Mar 18 '10 at 12:55
  • 1
    @John: well, in this case it’s *desirable* to lose the (deeply nested) stack trace of the recursive call so I think in this instance `throw ex` is actually *the right thing* to do. – Konrad Rudolph Mar 18 '10 at 12:56
  • 2
    @Konrad: @Rudzitis: I'm changing my reason for the -1. It's worse than screwing the stack by "throw ex". You're screwing the stack by artificially inducing extra stack levels through recursion at a time when stack depth actually matters. This is an iterative problem, not a recursive one. – John Saunders Mar 18 '10 at 13:04