peteraritchie / LongPath

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

Mono support incomplete #84

Closed Shlyonov closed 4 years ago

Shlyonov commented 4 years ago

Hello, I started using Pri.LongPath on Mono (Unity3D) about 3 years ago and ran into a problem: attribute DllImport("kernel32.dll") doesn't work on macOS (Linux). Then I had to write a check and an additional layer that would determine the OS type and choose Pri.LongPath for Windows and the default System.IO for everyone else (MacOS or Linux, i have tested only MacOS).

Yesterday I decided to update the Pri.LongPath version and founded that the mono check has added:

public static bool IsRunningOnMono() { return Type.GetType("Mono.Runtime") != null; }

Accordingly, Pri.LongPath has began to check the application framework and if mono, then use default System.IO. This means that on mono the library actually do nothing! We have all the same problems with long paths if app is running on mono and on windows, but the library do nothing!

Why does Pri.LongPath check the framework type instead of OS type? For example instead of "IsRunningOnMono()" Pri.LongPath could use "IsNotWindows()":

From https://www.mono-project.com/docs/faq/technical/

public static bool IsNotWindows() { var p = (int)Environment.OSVersion.Platform; return (p == 4) || (p == 6) || (p == 128); }

On windows, all calls to DllImport("kernel32.dll") work fine on both dotnet and mono, so we need an ability of using Pri.LongPath power on Mono+Windows too.

peteraritchie commented 4 years ago

Yesterday I decided to update the Pri.LongPath version and founded that the mono check has added:

public static bool IsRunningOnMono() { return Type.GetType("Mono.Runtime") != null; }

That was part of a contribution #73 which was interestingly to fix a problem with mono. I'll have a closer look at what's going on there.

@Shlyonov, what's the possibility of upgrading to .net core?

peteraritchie commented 4 years ago

@Shlyonov, I added a unix check in there in latest release (2.0.53), let me know if that works.

Shlyonov commented 4 years ago

@Shlyonov, what's the possibility of upgrading to .net core?

I think this is not necessary. .net core most used at servers (linux have no problem with long path, latest windows versions allows manual extending file path as i know, server developer could control path). But Pri could have .net standart 2.0 build which is compatible with latest .net framework and .net core. You will need to rewrite some of the code because file permissions method has changed as i know. I haven't tested long path problem with .net core may be MS has fixed it.

@Shlyonov, I added a unix check in there in latest release (2.0.53), let me know if that works.

@peteraritchie Thank you very much! I haven't time at the moment for testing it. I looked through your changes. I think it should works perfectly because i did exactly the same in my fork.

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.

peteraritchie commented 4 years ago

@peteraritchie Thank you very much! I haven't time at the moment for testing it. I looked through your changes. I think it should works perfectly because i did exactly the same in my fork.

Great! I've got #63 that I added a while ago to improve the implementation of mono support; but that should get you going in the meantime.

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.

Interesting, thanks for letting me know and the great detail. It's first I've heard of a dispose problem and I also expected the reader to dispose the stream. I'll look into that as well.

peteraritchie commented 4 years ago

fixed by #85