1

Basically I have some code like this that reads the contents of ASCII text file into a List:

    List<string> lines = new List<string> ( );
    using ( StreamReader sr = File.OpenText ( textfile ) )
    {
        string s = String.Empty;
        while ( ( s = sr.ReadLine ( ) ) != null )
            lines.Add ( s );
    }

But the thing is when the file is being written by another thread, it throws an exception:

The process cannot access the file 'myfile.txt' because it is being used by another process.

The same thing happens for File.ReadAllLines. Why do these functions lock the file on disk or care that the file is being use by another process?

I just want to read the contents periodically so if it's being used this time, then the next time it will read the updated content. I do this so check if new entries have been added as the user can also add them manually.

What functions I can use to read this file into memory without throwing an exception, or should I use run this code inside try/catch.

This is the latest code:

        var fs = new FileStream ( filename, FileMode.Open, FileAccess.Read, FileShare.ReadWrite );
        using ( StreamReader sr = new StreamReader ( fs ) )
        {
            string s = String.Empty;
            while ( ( s = sr.ReadLine ( ) ) != null )
                lines.Add ( s );
        }

The code that modifies the file:

public static void RemoveCoinFromBuyOrderLogs ( string symbol )
{
    if ( !walletLocked )
    {
        walletLocked = true;

        string [ ] lines = File.ReadAllLines ( walletFilename );

        var newlines = lines.Where ( c => !c.StartsWith ( symbol + "USDT" ) && !c.StartsWith ( symbol + "BUSD" ) && !c.StartsWith ( symbol + "USDC" ) && !c.StartsWith ( symbol + "TUSD" ) ).Select ( c => c ).ToList ( );
        File.WriteAllLines ( walletFilename, newlines );

        using ( FileStream fs = File.Open ( walletFilename, FileMode.OpenOrCreate ) )
        {
            StreamWriter sw = new StreamWriter ( fs );
            sw.AutoFlush = true;
            newlines.ForEach ( r => sw.WriteLine ( r ) );
        }

        walletLocked = false;
    }
}

public static void AddCoinToOrderLogs ( string newOrder, long orderId )
{
    if ( !walletLocked )
    {
        var lines = Utility.ReadAllLines ( walletFilename );
        lines = lines.Select ( line => line.Replace ( "\r", "" ) ).ToList ( );
        lines = lines.Where ( line => line != "" ).Select ( line => line ).ToList ( );

        var fields = lines.Select ( line => line.Split ( '\t' ) ).ToList ( );

        bool duplicate = false;
        foreach ( var field in fields )
        {
            if ( field.Length >= 5 )
            {
                long id = Convert.ToInt64 ( field [ 4 ] );
                if ( id == orderId )
                    duplicate = true;
            }
        }

        if ( !duplicate )
        {
            lines.Add ( newOrder );
            lines.Sort ( );

            walletLocked = true;
            File.WriteAllLines ( walletFilename, lines );
            walletLocked = false;
        }
    }
}
Joan Venge
  • 315,713
  • 212
  • 479
  • 689
  • _"should I run this code inside try/catch"_ that's always a good idea if you use `System.IO`. There are many things that can happpen and let it fail apart from the issue you have. – Tim Schmelter Mar 08 '21 at 14:20
  • Oh ok I thought it would cause a lot of overhead coz I am checking the file contents every second to see if the user entered new data, that's why. then i will use it. – Joan Venge Mar 08 '21 at 14:24
  • The process that is writing the file is part of your program? Because if not, I think the only safe way to read is to copy the file to a temp location and read the copy. If it is, you can set the permissions to allow other processes to read. – Magnetron Mar 08 '21 at 14:27
  • Yes some part of my program also modifies the file when necessary, but this function is to detect if the user added anything. You mean I can specify the write functions to allow others to read, i.e. not lock the file? – Joan Venge Mar 08 '21 at 14:29
  • 2
    @JoanVenge yes, use [this `FileStream` overload](https://learn.microsoft.com/en-us/dotnet/api/system.io.filestream.-ctor?view=net-5.0#System_IO_FileStream__ctor_System_String_System_IO_FileMode_System_IO_FileAccess_System_IO_FileShare_) and set [`FileShare`](https://learn.microsoft.com/en-us/dotnet/api/system.io.fileshare?view=net-5.0) to `Read` – Magnetron Mar 08 '21 at 14:31
  • 5
    @JoanVenge why don't you monitor file changes directly instead of checking every seconds? With a FileSystemWatcher f.ex. – XavierAM Mar 08 '21 at 14:32
  • @Magnetron ok I will check this now. – Joan Venge Mar 08 '21 at 14:36
  • @XavierAM I tried this but when I saved the file it didnt fire any event, so then I found out here about some limitation that it doesnt fire sometimes when the file is changed because windows is caching the result/date or something. – Joan Venge Mar 08 '21 at 14:36
  • @JoanVenge You should add the code that *writes* to the file as well, since the sharing modes must be compatible on the two sides. And it is not clear from the question whether the file is modified *only* from your program, or *also* from another external program. – dxiv Mar 27 '21 at 05:57
  • Ok I added it . – Joan Venge Mar 27 '21 at 06:11
  • 1
    Use FileShare.ReadWrite for readers and writers. You can also use this https://stackoverflow.com/a/5441631/403671 – Simon Mourier Mar 28 '21 at 10:22

3 Answers3

6

Take a look at this overload of File.Open(). It allows you to specify additional parameters to avoid the locking. I think it should do the trick.

For example, you can do var stream = new FileStream(textfile, FileMode.Open, FileAccess.Read, FileShare.ReadWrite);

mu88
  • 4,156
  • 1
  • 23
  • 47
3

First, if your application is multi-threaded, you shouldn't be using a bool guard. You should be using thread synchronization tools such as locks, mutexes, events and/or semaphores.

Also, your read is opening for share, but your writes aren't.

You are also not wrapping streams in using blocks. This is another problem. You should never do this:

StreamWriter sw = new StreamWriter(fs);

You should do this:

using(var sw = new StreamWriter(fs))
{
    // ...
}

The cardinal rule with objects that implement Dispose is you should always wrap them in a using block.

That aside, you probably don't want to read while writing or write while reading. That is going to give you massive problems with race conditions that will be hard to recreate when you need to debug what is going on.

Since you aren't using async/await, I would suggest using a lock. This will only allow one thread at a time to do file operations. No race conditions, no "sharing" files.

private static readonly object _fileLock = new object();

public static void RemoveCoinFromBuyOrderLogs(string symbol)
{
    lock(_fileLock)
    {
        var newlines = File.ReadAllLines(walletFilename)
            .Where(c =>
                !c.StartsWith(symbol + "USDT") &&
                !c.StartsWith(symbol + "BUSD") &&
                !c.StartsWith(symbol + "USDC") &&
                !c.StartsWith(symbol + "TUSD"));

        File.WriteAllLines(walletFilename, newlines);
    }
}

public static void AddCoinToOrderLogs(string newOrder, long orderId)
{
    lock (_fileLock)
    {
        var lines = File.ReadAllLines(walletFilename).ToList();
        lines = lines.Select(line => line.Replace("\r", "")).ToList();
        lines = lines.Where(line => line != "").Select(line => line).ToList();

        var fields = lines.Select(line => line.Split('\t')).ToList();

        bool duplicate = false;
        foreach (var field in fields)
        {
            if (field.Length >= 5)
            {
                long id = Convert.ToInt64(field[4]);
                if (id == orderId)
                    duplicate = true;
            }
        }

        if (!duplicate)
        {
            lines.Add(newOrder);
            lines.Sort();
            File.WriteAllLines(walletFilename, lines);
        }
    }
}

I can't test this code as I do not have the data to test, but try to get it to look close to that.

And, in all honesty, it is my opinion that you should be using something like an SQLite database to do this type of work. Manipulating a single flat file with multiple threads is a tough thing to do properly and efficiently.

ETA

Here is an example of an async/await pattern using SemaphoreSlim for synchronization

private static readonly SemaphoreSlim _smph = new SemaphoreSlim(1, 1);

private static async Task<IEnumerable<string>> ReadAllLinesAsync(
    string fileName, bool removeEmptyLines = true)
{
    using (var s = File.OpenText(fileName))
    {
        var data = await s.ReadToEndAsync().ConfigureAwait(false);
        return await Task.Run(() =>
            data.Split(new[] { Environment.NewLine },
                removeEmptyLines ? StringSplitOptions.RemoveEmptyEntries : StringSplitOptions.None));
    }
}

private static async Task WriteAllLinesAsync(string fileName, IEnumerable<string> lines)
{
    using (var s = File.OpenWrite(fileName))
    using (var sr = new StreamWriter(s))
    {
        var data = await Task.Run(() => 
            string.Join(Environment.NewLine, lines)).ConfigureAwait(false);
        await sr.WriteAsync(data);
    }
}

public static async Task RemoveCoinFromBuyOrderLogsAsync(string symbol)
{
    await _smph.WaitAsync().ConfigureAwait(false);
    try
    {
        var lines = await ReadAllLinesAsync(walletFilename);
        lines = lines.Where(c =>
                !c.StartsWith(symbol + "USDT") &&
                !c.StartsWith(symbol + "BUSD") &&
                !c.StartsWith(symbol + "USDC") &&
                !c.StartsWith(symbol + "TUSD"));
        await WriteAllLinesAsync(walletFilename, lines);
    }
    finally
    {
        _smph.Release();
    }
}

public static async Task AddCoinToOrderLogsAsync(string newOrder, long orderId)
{
    await _smph.WaitAsync().ConfigureAwait(false);
    try
    {
        var lines = await ReadAllLinesAsync(walletFilename);

        var duplicate = lines.Select(line => line.Split('\t'))
                .Any(x => (x.Length >= 5) && Convert.ToInt64(x[4]) == orderId);

        if (!duplicate)
        {
            var newLines = await Task.Run(() =>
            {
                var newList = lines.ToList();
                newList.Add(newOrder);
                newList.Sort();
                return newList;
            });

            await WriteAllLinesAsync(walletFilename, newLines);
        }
    }
    finally
    {
        _smph.Release();
    }
}

I added Task.Run on parts I thought could be CPU intensive operations.

Andy
  • 12,859
  • 5
  • 41
  • 56
  • Is it better to use async methods than? I use a single file because it keeps the log for buys and sells so it's very small file about 10-20KB, where I the app will log orders while another part of the code is using it to calculate the unit price so i can see if it's in profit, etc. – Joan Venge Mar 28 '21 at 05:32
  • @JoanVenge -- You don't need to use async/await since there is no synchronization context (i am assuming you are running this in a console project). I was picking your thread synchronization tool based on that fact. If you were using async/await, i would have suggested using `SemaphoreSlim` instead of `lock`. – Andy Mar 28 '21 at 15:48
  • It's a GUI app running "real-time" like every 20-50ms update and then WPF automatically updates bound data. – Joan Venge Mar 28 '21 at 15:55
  • @JoanVenge -- but, you have separate threads manipulating the data. I assume you do that so your UI doesn't lock up. If you move to async/await, then you'd need to rearchitect how you update your wallet file. I'll add an example of how to make this async/await. – Andy Mar 28 '21 at 16:44
  • Thanks a lot, yes I only have a BG worker executing the list of actions if the websockets triggers something like adding a new coin or removing, and then BG worker handles it. WPF UI uses a different thread to update its results. – Joan Venge Mar 28 '21 at 16:46
  • @JoanVenge -- ok, added that. Again, if you already have a BG worker, then async/await won't help. If you want to move it off the BG worker and call it from a thread that could be the UI thread, then you would use the async/await code. – Andy Mar 28 '21 at 16:48
  • Thanks a lot I will test this tomorrow. – Joan Venge Mar 28 '21 at 17:16
  • @JoanVenge -- If you have any issues, please let me know. I can't test it because I don't have the wallet data. If you give me the "wallet" data, I would be more than happy to optimize for thread-based and asynchronous operations. I have a lot of freetime lol – Andy Mar 28 '21 at 17:23
  • Thanks a lot appreciate it, I can definitely ask your advice when I am uncertain of some parts and maybe maybe a simplified version for you to check it out. It's nothing complicated. Cheers! – Joan Venge Mar 28 '21 at 17:26
1
File.ReadAllLines(StringPath)

This function opens a text file, reads all lines of the file, and then closes the file. If file is less than 4G could be useful.

Cast the result string array to a list and do what ever you want with it. The file remains free and unlocked. Update the final result, using WriteAllLines(path,StringLines) at the end.

There is also asynchronous method if it is really necessary to do it real-time This article could be really helpful. The functions are

For reading

public async Task SimpleReadAsync()
{
    string filePath = "simple.txt";
    string text = await File.ReadAllTextAsync(filePath);

    Console.WriteLine(text);
}

For writing

    public async Task SimpleWriteAsync()
    {
        string filePath = "simple.txt";
        string text = $"Hello World";
    await File.WriteAllTextAsync(filePath, text);

}
redParrot
  • 440
  • 1
  • 7
  • 14
  • Thanks how can I get a return form the Read Async? – Joan Venge Mar 28 '21 at 13:11
  • Read the article very carefully. Asynchronous IO has a lot of details, settings. But it's all . You'r not getting in to getting out. Whenever you need to read. That read all the text lines. that's all. – redParrot Mar 28 '21 at 18:35