peteraritchie / LongPath

drop-in library to support long paths in .NET
GNU Lesser General Public License v3.0
112 stars 43 forks source link

File handles not being disposed #86

Open peteraritchie opened 4 years ago

peteraritchie commented 4 years ago

Meanwhile i have found one bug (mono, unity3d). When i do some File IO operations with Pri - files handlers don't dispose. For example:

Pri.LongPath.File.WriteAllText(filePath, text);
Pri.LongPath.File.ReadAllText(filePath);

Write to file and then read file cause ioexception sharing violation on path. Same trouble with any Pri.LongPath.File.Open(...) Width default System.IO works fine.

It’s not disposing right i think.

Pri opens FileStream internaly by SafeFileHandle.

internal static FileStream Open(string path, FileMode mode, FileAccess access,
            FileShare share, int bufferSize, FileOptions options)
{
    ...

    SafeFileHandle handle = GetFileHandle(normalizedPath, mode, access, share, options);

    return new FileStream(handle, access, bufferSize, (options & FileOptions.Asynchronous) == 
    FileOptions.Asynchronous);
}

Then Pri creates StreamReader

private static StreamReader OpenText(string path, Encoding encoding)
{
    ...

    var stream = Open(path, FileMode.Open, FileAccess.Read, FileShare.Read, DefaultBufferSize, FileOptions.SequentialScan);
    return new StreamReader(stream, encoding, true, 1024);
}

And then at outer layer Pri has using statement

using (var streamReader = OpenText(path, encoding))
{
    return streamReader.ReadToEnd();
}

This statement should dispose StreamReader which should dispose FileStream which should dispose fileHandle... But i came to a conclusion that FileStream doesn't dispose fileHandle. I think its a special to mono issue, maybe to mono+unity3d (with not the latest version of mono).

I decided not to look for a problem in GetFileHandle.

internal static SafeFileHandle GetFileHandle(string normalizedPath, FileMode mode, FileAccess access, FileShare share, FileOptions options)

Its quite complicated and I needed a quick fix because we are developing an afpha of our product in a very short time.

And i did this:

public class FileStreamWithHandle : FileStream
{
    private readonly SafeFileHandle _fileHandle;

        ...ctors...

    // ctors with SafeFileHandle handle arg
    public FileStreamWithHandle(SafeFileHandle handle, FileAccess access) : base(handle, access)
     {
         _fileHandle = handle;
      }

    // and dispose override
    protected override void Dispose(bool disposing)
    {
        base.Dispose(disposing);

        if (disposing && _fileHandle != null)
            _fileHandle.Dispose();
     }
}

Replace FileStream with FileStreamWithHandle at Open method

internal static FileStream Open(string path, FileMode mode, FileAccess access,
            FileShare share, int bufferSize, FileOptions options)

And it works perfectly! But this solution isn't elegant (redurant for non mono) and I don’t know what exactly breaks dispose. It is necessary to understand the problem well in order to make a more correct solution.

Originally posted by @Shlyonov in https://github.com/peteraritchie/LongPath/issues/84#issuecomment-569244152

peteraritchie commented 4 years ago

@shlyonov WriteAllText definitely closes the safe handle. The unit test TestWriteAllText performs a WriteAllText followed by a ReadAllText without error; at least .NET Framework and Windows.

Shlyonov commented 4 years ago

@peteraritchie i've found this bug on mono (unity3d). I think that under .NET Framework everything work perfectly. Because it would be immediately noticeable. I have mono experience only with unity3d, this isn't the first thing that works as expected .NET Framework and bad on mono. You could try Pri on mono with unity, VS Installer assists with easy install it.