Matthew Goulart Matthew Goulart - 4 years ago 98
C# Question

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);
}
}

Answer Source

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);
Recommended from our users: Dynamic Network Monitoring from WhatsUp Gold from IPSwitch. Free Download