CODEWITHSUNDEEP
c#multithreadingparallel-processingtaskthreadpool
Matthew Goulart
Matthew Goulart

Reputation: 3065

Raising an event in a Task causes the subscriber method to fail

I have a few classes that are "linked" via various events. Basically a TCPListener, ClientSession and Server.

The TCPListener bubbles new connections up to the Server and then the Server creates a new ClientSession.

TCPListener passes the new socket via an event, ClientConnected.

The method raising the event looks like this:

    //Processes a new connection and immediately puts the listen socket back in a receiving state
    private void ProcessAccept(SocketAsyncEventArgs e)
    {
        ClientConnected(this, e));            

        StartAccept();
    }

I wanted to avoid doing anything that could prevent or slow down the accepting of new clients so I endeavored to raise the event with a Task. The reason for this is a user can override the OnClientConnected method in Server such that it is long running and could impact server performance.

Here is the revised method:

    //Processes a new connection and immediately puts the listen socket back in a receiving state
    private void ProcessAccept(SocketAsyncEventArgs e)
    {
        Task.Run(() => ClientConnected(this, e));            

        StartAccept();
    }

When the program is run with the new method, there are no exceptions on crashes, yet the server is unable to receive or send to the client.

Here is the OnClientConnected method in Server:

    /// <summary>
    /// An event that is fired when a client connects.
    /// </summary>
    /// <param name="sender">The Listener that accepted the connection</param>
    /// <param name="e">The SocketAsyncEventArgs </param>
    protected virtual void OnClientConnected(object sender, EventArgs e)
    {
        ClientSession Session = ClientSessionPool.Pop();

        Session.Socket = ((SocketAsyncEventArgs)e).AcceptSocket;

        string WelcomeMessage = "Connected";

        Session.SendAsync(Encoding.Default.GetBytes(WelcomeMessage));

        this.ClientSessions.Add(Session);

        Console.Write($"\rConnected clients:{ClientSessions.Count}");
    }

Simply reverting back to the old, synchronous method works just fine.

What is bugging me is that the SocketAsyncEventArgs that is passed through the event seems to be correct regardless of using Task or not and that is the only point of failure I can think of.

The SocketAsyncEventArgs seems to be completely wrong when using the Task version of the method.

I suspect it is my understanding of the stack allocated to a new thread that is causing my confusion. Can anyone see a hole in my logic? Thanks!

PS

I am aware that my current implementation of the ClientSessions list is NOT thread safe, but in my testing, I am only ever connecting with one client at a time. I will eventually fix that.

PPS Here is the StartAccept method in case it is useful:

    //Puts the accepting TCP socket back into an accepting state
    public void StartAccept()
    {
        // socket must be cleared since the context object is being reused
        m_SocketEventArgs.AcceptSocket = null;

        bool willRaiseEvent = m_Socket.AcceptAsync(m_SocketEventArgs);
        if (!willRaiseEvent)
        {
            ProcessAccept(m_SocketEventArgs);
        }
    }

Upvotes: 0

Views: 46

Answers (1)

Connell.O&#39;Donnell
Connell.O&#39;Donnell

Reputation: 3703

On solution would be to raise your event on the UI thread but run your event handling asynchronously.

private async void OnClientConnected(object sender, EventArgs e)
{
    await Task.Run(() => HandleClientConnected(object, e));
}

protected abstract Task HandleClientConnected(object sender, EventArgs e);

Upvotes: 2

Related Questions