views:

633

answers:

5

I've apparently worked myself into a bad coding habit. Here is an example of the code I've been writing:

using(StreamReader sr = new StreamReader(File.Open("somefile.txt", FileMode.Open)))
{
    //read file
}
File.Move("somefile.txt", "somefile.bak"); //can't move, get exception that I the file is open

I thought that because the using clause explicitly called Close() and Dispose() on the StreamReader that the FileStream would be closed as well. The only way I could fix the problem I was having was by changing the above block to this:

using(FileStream fs = File.Open("somefile.txt", FileMode.Open))
{
  using(StreamReader sr = new StreamReader(fs))
  {
    //read file
  }
}

File.Move("somefile.txt", "somefile.bak"); // can move file with no errors

Should closing the StreamReader in the first block also close the underlying FileStream? Or, was I mistaken?

Edit

I decided to post the actual offending block of code, to see if we can get to the bottom of this. I am just curious now.

I though I had a problem in the using clause, so I expanded everything out, and it still can't copy, every time. I create the file in this method call, so I don't think anything else has a handle open on the file. I've also verified that the strings returned from the Path.Combine calls are correct.

private static void GenerateFiles(List<Credit> credits)
{
    Account i;
    string creditFile = Path.Combine(Settings.CreditLocalPath, DateTime.Now.ToString("MMddyy-hhmmss") + ".credits");

    StreamWriter creditsFile = new StreamWriter(File.Open(creditFile, FileMode.Create));

    creditsFile.WriteLine("code\inc");

    foreach (Credit c in credits)
    {
        if (DataAccessLayer.AccountExists(i))
        {
            string tpsAuth = DataAccessLayer.GetAuthCode(i.Pin);
            creditsFile.WriteLine(String.Format("{0}{1}\t{2:0.00}", i.AuthCode, i.Pin, c.CreditAmount));
        }
        else
        {
            c.Error = true;
            c.ErrorMessage = "NO ACCOUNT";
        }

        DataAccessLayer.AddCredit(c);

    }

    creditsFile.Close();
    creditsFile.Dispose();

    string dest =  Path.Combine(Settings.CreditArchivePath, Path.GetFileName(creditFile));
    File.Move(creditFile,dest);
    //File.Delete(errorFile);
}
+11  A: 

Yes, StreamReader.Dispose closes the underlying stream (for all public ways of creating one). However, there's a nicer alternative:

using (TextReader reader = File.OpenText("file.txt"))
{
}

This has the added benefit that it opens the underlying stream with a hint to Windows that you'll be accessing it sequentially.

EDIT: Here's my test app which shows the first version working for me. I'm not trying to say that's proof of anything in particular - but I'd love to know how well it works for you.

using System;
using System.IO;

class Program
{
    public static void Main(string[] args)
    {
        for (int i=0; i < 1000; i++)
        {
            using(StreamReader sr = new StreamReader
                  (File.Open("somefile.txt", FileMode.Open)))
            {
                Console.WriteLine(sr.ReadLine());
            }
            File.Move("somefile.txt", "somefile.bak");
            File.Move("somefile.bak", "somefile.txt");
        }
    }
}

If that works, it suggests that it's something to do with what you do while reading...

EDIT: Here's a shortened version of your edited question code - which again works fine for me, even on a network share. Note that I've changed FileMode.Create to FileMode.CreateNew - as otherwise there could still have been an app with a handle on the old file, potentially. Does this work for you?

using System;
using System.IO;

public class Test
{    
    static void Main()
    {
        StreamWriter creditsFile = new StreamWriter(File.Open("test.txt", 
                                          FileMode.CreateNew));

        creditsFile.WriteLine("code\\inc");

        creditsFile.Close();
        creditsFile.Dispose();

        File.Move("test.txt", "test2.txt");
    }
}
Jon Skeet
Any idea why I can't move the file in the first block?
scottm
No - that should be okay. Odd.
Jon Skeet
Does it *always* fail with the first version and *always* work with the second?
Jon Skeet
@jon, yes. like clock work.
scottm
Just tried the first version and it worked for me 20 times in a loop.
Jon Skeet
I've coded it the first way a hundred times, and never had a problem before. Now I'm opening the file on a windows share and this was the only way I could resolve the problem. It failed every time with the first example, and has worked every time with the second. So I thought I was doing it wrong
scottm
I can confirm Jon's observation, all three ways work. There must be something else affecting this.
Samuel
I just tested on a Windows share and all three ways worked again.
Samuel
Just tried it against a network share (albeit the share was a Linux box) and it worked okay then. The only difference is that in one case the stream is closed first, then the StreamReader. But StreamReader.Dispose will definitely call Stream.Close. Very weird.
Jon Skeet
I edited the question and added the actual block of code where this is happening (all names have been changed to protect the innocent). Maybe someone can shed some light on the flaw in my logic.
scottm
Is it possible that the *destination* file already exists?
Jon Skeet
Just tried your example on the actual share this is happening on and there were no problems. After the app closes I can manually move the file with no problems
scottm
If you could keep reworking the code until you've got a short but *complete* program which demonstrates the problem (no need for actual credit stuff, unless something in there might touch the file) that's bound to help.
Jon Skeet
I think I'm done with it. I just wanted to make sure I wasn't completely wrong in assuming the FileStream was closed. Since the second block fixed the problem and it's not that important, I'm not going to delve any deeper.
scottm
Hmm... that sounds like a recipe for trouble later though. Any time you can't explain why one way works and another doesn't, that usually means there's a problem somewhere else which will exhibit itself differently later on. Ah well... it really *should* work. Does File.OpenText work btw?
Jon Skeet
Haven't tried that, maybe I will make some tests tomorrow and see if some of the other suggestions here work.
scottm
+1. @scotty dispose calls the inner stream.Close, which in turns calls its Dispose. I checked with reflector after JS comments on my answer (I deleted it, since it was wrong/misleading :().
eglasius
What's your share hosted on, and what's your client OS? There can be some slight differences in cached writes between different setups of Windows (not to mention Samba, Netware, etc.). IIRC, there's a registry key to control it in Win2K+.
Mark Brackett
@Mark, it's a windows 2000 server machine.
scottm
A: 

Note - your using blocks do not need to be nested in their own blocks - they can be sequential, as in:

using(FileStream fs = File.Open("somefile.txt", FileMode.Open))
using(StreamReader sr = new StreamReader(fs))
{
    //read file
}

The order of disposal in this case is still the same as the nested blocks (ie, the StreamReader will still dispose before the FileStream in this case).

Not Sure
It looks better the other way to me.
scottm
A: 

Is there any possibility that something else has a lock to somefile.txt?

A simple check from a local (to the file) cmd line

net files

may well give you some clues if anything else has a lock.

Alternatively you can get something like FileMon to take even more details, and check that your app is releasing properly.

Zhaph - Ben Duguid
+1  A: 

I would try to use FileInfo.Open() and FileInfo.MoveTo() instead of File.Open() and File.Move(). You could also try to use FileInfo.OpenText(). But these are just suggestions.

MartinStettner
I'll try the FileInfo methods, but I think underneath, they are just calls to File.Move
scottm
Yep, but perhaps they handle the stream (being created by FileInfo.Open()) internally. Also, documentation says they do some security checks only once, so it might be slightly faster ...
MartinStettner
A: 

Since this doesn't seem to be a coding issue, I'm going to put my syadmin hat on and offer a few suggestions.

  1. Virus scanner on either the client or server that's scanning the file as it's created.
  2. Windows opportunistic locking has a habit of screwing things up on network shares. I recall it being mostly an issue with multiple read/write clients with flat file databases, but caching could certainly explain your problem.
  3. Windows file open cache. I'm not sure if this is still a problem in Win2K or not, but FileMon would tell you.

Edit: If you can catch it in the act from the server machine, then Sysinternal's Handle will tell you what has it open.

Mark Brackett
Very interesting. I've also had problems in the past in another application that I didn't code, but we found the culprit to be AVG. I'll have to look into that.
scottm