1

I wrote a Client/Server async classes which works fine in the console. I created a WinForm project for the server which subscribes to an event thrown by the server when there is a .Pending() connection and writes some messages into a textbox - which causes a Cross-Thread exception. The exception does not surprise me, however I am looking for a way to invoke that event without causing this exception, without handling it on the GUI/Control with .InvokeRequired and .Invoke - if that is even possible?

The server is started like that:

Server server = new Server(PORT);
server.RunAsync();

in .RunAsync() i just iterate over the network devices and set them to listening and invoke an event that the server has started, this also writes into the GUI however without any issue.

public async Task RunAsync()
    {
        GetNetworkDevicesReady(Port);

        await Task.Factory.StartNew(() =>
        {
            Parallel.ForEach(networkListeners, (listener) =>
            {
                Write.Info($"LISTENING ON {listener.LocalEndpoint}");
                listener.Start();
            });
        });
        IsRunning = true;

        OnServerStarted?.Invoke(this, networkListeners.Where(l=>l.Active).ToList());

    }

The code below is registered on the Form.Load event and does not cause a Cross-Thread exception when writing "SERVER STARTED" in the textbox.

server.OnServerStarted += (s, a) =>
        {
            consoleWindow1.Event("SERVER STARTED", $"{Environment.NewLine}\t{string.Join($"{Environment.NewLine}\t", a.Select(x=>x.LocalEndpoint))}");

            consoleWindow1.Event("WAITING FOR PENDING CONNECTIONS");
            server.WaitForConnectionsAsync();
        };

And this is the code which runs indefinite until a cancellation token is triggered:

public async Task WaitForConnectionsAsync()
    {
        waitingForConnectionsToken = new CancellationTokenSource();

        await (waitinfConnectionTaks=Task.Factory.StartNew(async () =>
        {
            while (!waitingForConnectionsToken.IsCancellationRequested)
            {
                foreach (var listener in networkListeners)
                {
                    if (waitingForConnectionsToken.IsCancellationRequested) break;

                    if (!listener.Active)
                    {
                        continue;
                    }

                    if (listener.Pending())
                    {
                        try
                        {
                            TcpClient connection = await listener.AcceptTcpClientAsync();
                            //TODO: need to send it synchronised, since this causes a Cross-Thread when using WinForms
                            OnPendingConnection?.Invoke(this, connection);

                        }
                        catch (ObjectDisposedException x)
                        {
                            Write.Error(x.ToString());
                        }

                    }
                }
            }
        }));
    }

I know I can use the textbox .InvokeRequired and .Invoke on the GUI but I have the feeling that the server should throw the event in a way the GUI doesn't cause a Cross-Thread exception.

Is there a way to invoke the eventhandler in that "infinite task" without causing this exception?

ProgrammingLlama
  • 36,677
  • 7
  • 67
  • 86
Tomek
  • 323
  • 1
  • 4
  • 19
  • 2
    "but I have the feeling that the server should throw the event in a way the GUI doesn't cause a Cross-Thread exception" - In non-UI contexts, synchronising to a single thread could slow things down. Are you sure this is what you want to do? – ProgrammingLlama Oct 22 '18 at 04:43
  • 2
    async/await will synchronize to the calling synchronization context after await if possible and you did not use ConfigureAwait(false). But each listener is started inside the Parallel.ForEach which also runs inside a task worker thread. So there is no sync context anymore the listener can sync to, thus you get the cross thread exception – Sir Rufo Oct 22 '18 at 05:10
  • 2
    Don't use `Parallel.Forxxx` with `async/await` –  Oct 22 '18 at 06:06
  • 1
    `I am looking for a way to invoke that event without causing this exception, without handling it on the GUI/Control with .InvokeRequired and .Invoke` If that's what you want, this is the purpose of the synchronization context. Capture the current synchronization context in `RunAsync`, then use it in `WaitForConnectionsAsync` to execute the event handlers. But I'd take John's advice in consideration first – Kevin Gosse Oct 22 '18 at 06:31
  • "server should throw the event in a way ..." could only happen if the server used something like SomeControl.Invoke(). Binding it to WinForms and slowing it down. – bommelding Oct 22 '18 at 10:03
  • @MickyD any elaboration why? Is that doubling up like creating a thread for a thread? – Tomek Oct 22 '18 at 22:45
  • @John & Kevin of course I wouldn't want it to slow down (not at least in the server class). After sleeping and reading the comments - a solution would be to not create an infinite loop around it and making it awaitable. After that has completed, invoking the event and then re-creating / starting the task again (until some exit/stop condition) – Tomek Oct 22 '18 at 22:48
  • @bommelding yes, however I wanted to avoid passing in any forms/controls. I might have somehow found out through the invocation list where the call came from and check if it was a control/form and then call Invoke on it.. but again I hoped to avoid that. – Tomek Oct 22 '18 at 23:14
  • 1
    https://stackoverflow.com/a/11565317/585968 and https://blogs.msdn.microsoft.com/pfxteam/2009/05/26/does-parallel-for-use-one-task-per-iteration/ and the big one https://stackoverflow.com/a/42450935/585968 –  Oct 22 '18 at 23:53
  • @MickyD thanks for the links, will go through them. – Tomek Oct 23 '18 at 01:03

1 Answers1

0

Thanks to the comments and a good portion of sleep I solved my issue by changing the WaitForConnectionsAsync to the following code:

List<TcpClient> connections = new List<TcpClient>();
public async Task WaitForConnectionsAsync()
{
        await (waitinfConnectionTaks = Task.Factory.StartNew(async () =>
        {
           //REMOVED LOOP
           // while (!waitingForConnectionsToken.IsCancellationRequested)
            {
                foreach (var listener in networkListeners)
                {
                    if (waitingForConnectionsToken.IsCancellationRequested) break;

                    if (!listener.Active)
                    {
                        continue;
                    }

                    if (listener.Pending())
                    {
                        try
                        {
                            TcpClient connection = await listener.AcceptTcpClientAsync();

                            //RETAIN CONNECTIONS IN A LIST
                            connections.Add(connection);
                        }
                        catch (ObjectDisposedException x)
                        {
                            Write.Error(x.ToString());
                        }

                    }
                }
            }
        }));
        //ITERATE OVER CONNECTIONS
        foreach (var connection in connections)
        {
            //INVOKE EVENT
            OnPendingConnection?.Invoke(this, connection);
        }
        //CLEAR THE LIST
        connections.Clear();

        //RESTART THE TASK
        if(!waitingForConnectionsToken.IsCancellationRequested)
           WaitForConnectionsAsync();
    }

So Basically i am catching all pending connections into a list, once the work has completed, I run over the list, fire the event with each connection, clear the list and then start the task again. This code change doesn't throw the Cross-Thread exception anymore.

An improvement i could add now is to accept a collection in the event instead a single connection.

If you have any improvements or better practice suggestions, please let me know.

Tomek
  • 323
  • 1
  • 4
  • 19