14

I just saw this question: Is it safe to use static methods on File class in C#?. To summarize OP has an IOException because file is in use in this ASP.NET code snippet:

var text= File.ReadAllText("path-to-file.txt");
// Do something with text
File.WriteAllText("path-to-file.txt");

My first thought has been it's a simple concurrent access issue because of multiple ASP.NET overlapping requests. Something I'd solve centralizing I/O into a synchronized thread-safe class (or dropping files in favor of something else). I read both answers and when I was about to downvote one of them then I saw who those users are and I thought what the h* and stopped.

I'll cite them both (then please refer to original answers for more context).

For this OP paragraph:

I am guessing that the file read operation sometimes is not closing the file before the write operation happens [...]

An answer says:

Correct. File systems do not support atomic updates well [...] Using FileStream does not help [...] File has no magic inside. It just uses FileStream wrapped for your convenience.

However I don't see any expectancy for an atomic operation (read + subsequent write) and parallel (because of partially overlapping multi-threaded requests) may cause concurrent accesses. Even an atomic I/O operation (read + write) will have exactly same issue. OK FileStream may be asynchronous but it's not how File.ReadAllText() and File.WriteAllText() use it.

The other answer made me much more perplex, it says:

Although according to the documentation the file handle is guaranteed to be closed by this method, even if exceptions are raised, the timing of the closing is not guaranteed to happen before the method returns: the closing could be done asynchronously.

What? MSDN says method will open, read and close file (also in case of exceptions). Is it ever possible that such method will close file asynchronously? Will OS defer CloseHandle()? In which cases? Why?

In short: is it just a misunderstanding or CloseHandle() is asynchronous? I'm missing something extremely important?

Community
  • 1
  • 1
Adriano Repetti
  • 65,416
  • 20
  • 137
  • 208
  • I disagree with the answer in that thread. When the Windows API returns from `CloseHandle()`, that handle IS closed. If there's an exception, it's because more than one thread is accessing it at the same time. Since `File.ReadAllLines()` winds up closing the file using `CloseHandle()`, the file WILL be closed when it returns. If this wasn't true, all hell would break loose. I'm pretty sure the error mentioned in the other question is due to multithreading issues. – Matthew Watson Sep 07 '15 at 08:04
  • @MatthewWatson I agree with you, it would break half of our code out there. I'm asking just because those users rep (in case it's a remote undocumented weird corner case). – Adriano Repetti Sep 07 '15 at 08:07
  • In fact if you consider a common pattern used for log files (open, append, flush, close) where very frequent updates occur, things would turn nasty very quickly if you couldn't guarantee you can reopen a file shortly after closing it. (There may be instances involving other kinds of handles where this doesn't work the same, but for basic file handles with a single instance open, they will be closed deterministically.) – Matthew Watson Sep 07 '15 at 08:10
  • @MatthewWatson As long as there is only one handle, and it's synchronous, you are right. `FileStream` will only have one handle, so that's fine, and it will also be synchronous for `File.ReadXXX`. The common pattern used for log files you mention *is wrong* (the redundant flush before closing being one tiny example) - it just isn't wrong enough to fail completely at all times. That's the worst kind of wrong as far as maintainability goes. That said, the problem is somewhere else in this case - something else is touching the same file. This is *very* common in ASP.NET. – Luaan Sep 07 '15 at 08:25
  • @Luaan I had in my mind working at the Windows API level, so often you're doing your own buffering then, in which case flushing is necessary. I guess you're thinking of the C# File class. (I use NLog for my logging btw) – Matthew Watson Sep 07 '15 at 08:35
  • @MatthewWatson Oh, sure - in that case, you do need to flush the buffers manually. We are dealing with a C#/.NET question, so I assumed we're talking about the same thing :) .NET `FileStream` flushes automatically on `Close`/`Dispose`, and actually tries to do an asynchronous flush if possible. – Luaan Sep 07 '15 at 08:44
  • @Luaan But this is very much a question about the Windows API level, if you think about it. (Or at least, the answer has to be very much about the Windows API level.) – Matthew Watson Sep 07 '15 at 08:52
  • @Luaan here I agree with Matthew, FileStream class introduces its own complexity but it's not asynchronous unless you explicitly ask for it (and it's not what File.ReadXyz() does). – Adriano Repetti Sep 07 '15 at 09:33
  • 2
    The file system is evil and conspires to break your code at every turn. When confronted with the file system running away screaming like a little girl is usually the best approach. – Martijn Sep 07 '15 at 14:58

2 Answers2

4

If you look at the CloseHandle documentation, it states that each method which opens a handle has a description of how it should be closed:

The documentation for the functions that create these objects indicates that CloseHandle should be used when you are finished with the object, and what happens to pending operations on the object after the handle is closed. In general, CloseHandle invalidates the specified object handle, decrements the object's handle count, and performs object retention checks. After the last handle to an object is closed, the object is removed from the system.

When you look at the CreateFile docs, this is what it says:

When an application is finished using the object handle returned by CreateFile, use the CloseHandle function to close the handle. This not only frees up system resources, but can have wider influence on things like sharing the file or device and committing data to disk.

I would find it peculiar that CloseHandle would yield that the underlying handle is closed while asynchronously retaining the file for additional checks. This would weaken many guarantees the OS makes to the callers, and would be a source for many bugs.

Yuval Itzchakov
  • 146,575
  • 32
  • 257
  • 321
  • 6
    Indeed. The notion that the following code can fail (unless some other thread is accessing the same file) is ridiculous: `while (true) { File.WriteAllText("test.txt", "1\n2\n3\n"); File.ReadAllText("test.txt"); }` – Matthew Watson Sep 07 '15 at 08:21
1

The first two quotes in your question are not supposed to be related. When File.* is done, or when you close a FileStream, the file is unlocked immediately. There never is any kind of "lingering". If there was you could never safely access the same file again without rebooting.

May answer assumes that the code in the question is being run multiple times in parallel. If not, that code is clearly safe.

However I don't see any expectancy for an atomic operation ... Even an atomic I/O operation (read + write) will have exactly same issue.

That's true. I don't know why I made a statement about that in my answer (it's correct, though. Just not relevant).

the timing of the closing is not guaranteed to happen before the method returns: the closing could be done asynchronously.

I don't know why he said that because it's not correct under any circumstances that I can think of. Closing a handle has an immediate effect.

I think your understanding of the situation is completely accurate. Apparently, our answers were unclear and slightly misleading... Sorry about that.

usr
  • 168,620
  • 35
  • 240
  • 369
  • Closing a handle has an immediate effect - as long as there's only one user of that handle. That is always the case with the static methods on `File`, though (and most of the other managed APIs for that matter). – Luaan Sep 07 '15 at 10:13