Is `Plc.ReadClassAsync` thread safe?

[NullRefer]

S7NetPlus Version: 0.12.0

I have one plc instance in whole application, and open it at start of application. Then, I start to monitor up to 10 plc DataBlocks which is all the same structure with different threads. But sometimes (may be running for a day or two), it will throw exception which indicate the bytes read error as below:

S7.Net.PlcException: Received 38 bytes: '32-03-00-00-00-00-00-02-00-18-00-00-04-01-FF-04-00-A0-04-00-00-00-00-00-00-00-00-00-00-00-00-00-00-00-00-00-00-00', expected 372 bytes.

And then the byte[] to string conversion fails with exception threw by S7String.cs: https://github.com/S7NetPlus/s7netplus/blob/6aa0133081048131aa269d0d5b780ec7fe01028f/S7.Net/Types/S7String.cs#L40 https://github.com/S7NetPlus/s7netplus/blob/6aa0133081048131aa269d0d5b780ec7fe01028f/S7.Net/Types/S7String.cs#L49-L51

the application can simplified as the code shows

// main
static async Task Main(string[] args)
{
    using var plc = new Plc(CpuType.S71500, "127.0.0.1", 102, 0, 1);
    await plc.OpenAsync();
    var dbList = new int[] { 100, 200, 300, 400, 500 };
    var tasks = dbList.Select(db => MonitorDataBlockAsync(plc, db));
    await Task.WhenAll(tasks);
}

// monitor dataBlock
private static async Task MonitorDataBlockAsync(Plc plc, int db, CancellationToken token = default)
{
    while (true)
    {
        token.ThrowIfCancellationRequested();
        try
        {
            var result = await plc.RetrieveFromPlcAsync<HmiInfo>(db);
            System.Console.WriteLine(result.ToString());
        }
        catch (Exception exc)
        {
            System.Console.WriteLine(exc);
        }
        finally
        {
            await Task.Delay(1000, token);
        }
    }
}

// the HmiInfo class
public class HmiInfo : PtoBase
{
    public byte[] ByteArr1 { get; set; } = new byte[500];
    public string ByteArr1String => S7String.FromByteArray(ByteArr1);

    public byte[] ByteArr2 { get; set; } = new byte[500];
    public string ByteArr2String => S7String.FromByteArray(ByteArr2);

    public override int Offset => 0;

    public override string ToString() => $"ByteArr1: {ByteArr1String}, ByteArr2: {ByteArr2String}";
}

public abstract class PtoBase
{
    public abstract int Offset { get; }

    public virtual string ToJson()
    {
        var result = JsonConvert.SerializeObject(this, Formatting.Indented, new JsonSerializerSettings()
        {
            ContractResolver = new ByteContractResolver(),
        });
        return result;
    }
}

// plc extensions
public static class S7NetPlusExtensions
{
    public static async Task<T> RetrieveFromPlcAsync<T>(this Plc plc, int db) where T : PtoBase, new()
    => await plc.RetrieveFromPlcWithOffsetAsync<T>(db, new T().Offset);

    public static async Task<T> RetrieveFromPlcWithOffsetAsync<T>(this Plc plc, int db, int offset, int timeoutMs = 0) where T : PtoBase, new()
    {
        if (timeoutMs == 0) timeoutMs = 5000;
        async Task<T> ReadAsync(Plc p) => await p.ReadClassAsync<T>(db, offset, new CancellationTokenSource(timeoutMs).Token);

        return await OperatePlcWithRetryAsync(plc, async p => await ReadAsync(plc));
    }

    private static async Task<T> OperatePlcWithRetryAsync<T>(Plc plc, Func<Plc, Task<T>> operation)
    {
        var i = 0;
        while (i++ < 5)
        {
            try
            {
                return await operation(plc);
            }
            catch (Exception e)
            {
                System.Console.WriteLine(e);
            }
        }
        System.Console.WriteLine("Error after 5 retries");
        throw new Exception();
    }
}

[NullRefer]

It occurs when multiple threads use one plc instance with no lock. But the exception did not happen when I use ReadClassAsync rather than self-defined RetrieveFromPlcAsync which wraps the method.

[mycroes]

It's not thread safe, none of the read/write methods are. You should choose a synchronization method that suits your usage.

[NullRefer]

It's not thread safe, none of the read/write methods are. You should choose a synchronization method that suits your usage.

I notice that the TaskQueue was introduced into v0.12.0, and I thought it was designed to solve the concurrency problem.

https://github.com/S7NetPlus/s7netplus/blob/5318f94dd71b960c0771c4bb57615e3b42e7ee09/S7.Net/Internal/TaskQueue.cs

[mycroes]

It's not thread safe, none of the read/write methods are. You should choose a synchronization method that suits your usage.

I notice that the TaskQueue was introduced into v0.12.0, and I thought it was designed to solve the concurrency problem.

https://github.com/S7NetPlus/s7netplus/blob/5318f94dd71b960c0771c4bb57615e3b42e7ee09/S7.Net/Internal/TaskQueue.cs

I'm sorry, you're absolutely correct. I had already forgotten about adding that.

I'll look into this, perhaps from the received message I can deduce something. As far as I can tell the TaskQueue should prevent concurrent calls to the PLC, but maybe there's something I overlooked.

[NullRefer]

There is some concurrency problem when I use one plc instance in multiple threads. The typical one that I cannot figure out is that the bytes is out of order. That is, the bytes for Class A was filled in Class B! I'm planning to new one plc for each thread to avoid this.

Looking forward your reply~ Thanks!