Asynchronous components and WinForms

Question

I'm currently writing a component to communicate with an Ethernet based device and am having to use asynchronous sockets. At times when I receive specific 'commands' from the device, I need to raise an event for whatever program is using my component (most usually a WinForm.) I'm creating a sample form for the user but I am having difficulty allowing the client form to receive the events and modify the form; I'm getting the typical "Cross-thread operation not valid: Control 'listStrings' accessed from a thread other than the thread it was created on."

I've tried reading over Implementing the Event-based Asynchronous Pattern, and Walkthrough: Implementing a Component That Supports the Event-based Asynchronous Pattern, though it doesn't quite seem to be exactly what I need, especially when reading "Opportunities for Implementing the Event-based Asynchronous Pattern" in the first link.

.Net / C# is more of a hobby than profession, and in this project - this is the last piece I need to figure out before being able to complete it. Would it be better to use a "thread-safe" (I know, everyone throws that term around like it only means one thing) existing TCP/IP component rather than trying to implement it myself?

EDIT: Here's my network class code to show you how I'm implementing it now. I forget where I came across this snippet, but it's worked fine up until I've added the form.

internal class Network
{
    private Device dev;
    private TcpClient client;
    private NetworkStream ns;
    private byte[] buffer = new byte[2048];
    private Queue<byte[]> _msgQ = new Queue<byte[]>();

    public Network(Device d)
    {
        dev = d;
    }

    internal void Connect(string ipAddress, int port)
    {
        client = new TcpClient();
        client.BeginConnect(ipAddress, port, new AsyncCallback(OnConnect), null);
    }

    internal byte[] getLocalIp()
    {
        return ((IPEndPoint)client.Client.LocalEndPoint).Address.GetAddressBytes();
    }

    private void OnConnect(IAsyncResult ar)
    {
        try
        {
            client.EndConnect(ar);
            ns = new NetworkStream(client.Client);
            ns.BeginRead(buffer, 0, 2048, new AsyncCallback(OnRead), null);
            while (_msgQ.Count > 0)
            {
                byte[] message = _msgQ.Dequeue();
                ns.Write(message, 0, message.Length);
            }
            dev.dvDevice._connected = true;
        }

        catch (Exception ex)
        {
            Console.WriteLine(ex.Message);
        }
    }

    internal void Disconnect()
    {
        try
        {
            client.Close();
            dev.dvDevice._connected = false;
        }

        catch (Exception ex)
        {
            Console.WriteLine(ex.Message);
        }
    }

    internal void Write(byte[] message)
    {
        if ((!client.Connected) || ns == null)
        {
            _msgQ.Enqueue(message);
            return;
        }
        ns.Write(message, 0, message.Length);
    }

    private void OnWrite(IAsyncResult ar)
    {
        try
        {
            ns.EndWrite(ar);
        }

        catch (Exception ex)
        {
            Console.WriteLine(ex.Message);
        }
    }

    private void OnRead(IAsyncResult ar)
    {
        try
        {
            int recv = ns.EndRead(ar);
            byte[] message = new byte[recv];
            Buffer.BlockCopy(buffer, 0, message, 0, recv);
            dev.dvDevice._mh.Parse(message);
            ns.BeginRead(buffer, 0, 2048, new AsyncCallback(OnRead), null);
        }

        catch (Exception ex)
        {
            Console.WriteLine(ex.Message);
        }
    }
}

Device is the class which is exposed to the client. It contains a MessageHandler (_mh) class which does all the parsing. Device contains the public event which is called by the MessageHandler on specific responses. Hopefully this helps in what I have so far; I'd prefer not having to rewrite too much, but to make it right (and work properly), I will if I must.

EDIT (2): My goal for this library is that the user should not at all have to manage any of the threads - so when an event is raised, say "ReceiveString", the user should just be able to act on it without any thought.

EDIT (3): More code for completeness.

public delegate void OnStringEvent(byte[] str);

public class Device
{
    internal struct _device
    {
        // other stuff too, but here's what's important
        public bool _connected;
        public bool _online;
        public MessageHandler _mh;
        public Network _net;
    }

    public event  OnStringEvent OnString;

    internal void ReceiveString(byte[] str)
    {            
        OnString(str);
    }

    internal _device dvDevice;
    public Device(int device_number, int system_number)
    {
        dvDevice = new _device(device_number, system_number);
        dvDevice._mh = new MessageHandler(this);
        dvDevice._net = new Network(this);
    }
}

internal class MessageHandler
{
    private Device dev;

    public MessageHandler(Device d)
    {
        dev = d;
    }

    public void Parse(byte[] message)
    {
        // The code goes through the message and does what it needs to
        // and determines what to do next - sometimes write back or something else

        // Eventually if it receives a specific command, it will do this:
        dev.ReceiveString(ParseMessage(ref _reader));
     }
}

Answer 1

I like @Polity's answer, being an Rx fan I would say use Rx (Reactive Extensions)

 //we convert a typical begin/end (IAsyncPattern) into an observable sequence
 //it returns a Func -read() that takes a byte, two ints and returns one.
 var read = Observable.FromAsyncPattern<byte[], int, int, int>
                            (networkStream.BeginRead, networkStream.EndRead)
.ObserveOn(Scheduler.Dispatcher);

// Now, you can get an IObservable instead of an IAsyncResult when calling it.
byte[] someBytes = new byte[10];
IObservable<int> observable = read(someBytes, 0, 10);

observable.Subscribe(x=> 
//x will be the returned int. You can touch UI from here.
);

---

Based on your code I can see that another thread calls the OnString event, then I assume when you subcribe to it, you're just adding the string into the listbox.

device.OnString += new OnStringEvent(device_onstring);

void device_onstring(byte[] str)
{ 
 listStrings.Items.Add(...);//this is wrong, will give cross thread op error.
 //you do this: 
 this.Invoke(new MethodInvoker(delegate()
   {
       listStrings.Items.Add(..);
       //or anything else that touches UI
   });
 // this should refer to a form or control.
}

Answer 2

Do youself a favor and rely on TPL to do the synchronization lifting for you. Example:

NetworkStream stream = MySocket.NetworkStream;

// creat a Task<int> returning the number of bytes read based on the Async patterned Begin- and EndRead methods of the Stream
Task<int> task = Task<int>.Factory.FromAsync(
        fs.BeginRead, fs.EndRead, data, 0, data.Length, null);

// Add the continuation, which returns a Task<string>. 
return task.ContinueWith((task) =>
{
    if (task.IsFaulted)
    {
        ExceptionTextBox.Text = task.Exception.Message;
    }
    else 
    {
        ResultTextBox.Text = string.Format("Read {0} bytes into data", task.Result);
    }
}, TaskScheduler.FromCurrentSynchronizationContext());

Answer 3

This should be a pretty easy problem to solve: you just need to execute any code in your form that updates controls using Invoke.

The precise implementation will depend on how the async code is calling back into your form. If you add that code to your question, we can provide a more complete answer.

Answer 4

You can handle this in 2 places depending on your design. If the event is raised from a different thread you can handle it in the event handler by checking the .invokeReqeuired property of the form( or other control) handling the event. If it returns true you should use the .beginInvoke method to marshal the call to the proper thread.

Depending on your design you can handle it from the other end by passing your component an instance of the form you want to marshal to. Before you raise the event, check .invokeRequired and marshal the call so that the event is raised in the proper thread. This way the code using your library doesn't have to worry about threads, but it required that your library have a reference to system.windows.forms.