Skip to content

Fixed listener loop dying on ChannelClosedException#4

Merged
miniduikboot merged 3 commits into
Impostor:masterfrom
ykundesu:fix/ChannelClosedException
Jun 27, 2026
Merged

Fixed listener loop dying on ChannelClosedException#4
miniduikboot merged 3 commits into
Impostor:masterfrom
ykundesu:fix/ChannelClosedException

Conversation

@ykundesu

Copy link
Copy Markdown
Contributor

In UdpConnectionListener, the try/catch block surrounding the receiving while loop is located outside the loop. If ProcessData throws an exception, the entire loop unwinds, logging "Listen loop error" once before ListenAsync terminates. Since there is no restart mechanism and there is only one loop per shared UDP socket, all lobbies on the server become unable to receive packets.
While this usually has no impact, a defect occurs in very rare cases.

The trigger is a race condition within ProcessData:

  1. Retrieve the connection from the dictionary (TryGetValue)
  2. await client.Pipeline.Writer.WriteAsync(...)

If the connection in question is disposed between steps 1 and 2 (due to a kick, leaving, or a keep-alive timeout calling Pipeline.Writer.Complete()), step 2 results in a write to a closed channel, throwing a ChannelClosedException.


// Write to client.
await client.Pipeline.Writer.WriteAsync(data.Buffer);
client.Pipeline.Writer.TryWrite(data.Buffer);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure what the advantage of this change is. If TryWrite could not write the results back, don't we need to handle this?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The intention was to ensure that the loop does not stop even if an issue occurs.
In fact, I believe there is no problem because TryWrite returns false only when the write operation has already been completed.
https://github.com/dotnet/runtime/blob/main/src/libraries/System.Threading.Channels/src/System/Threading/Channels/UnboundedChannel.cs#L256

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah I see, it returns false when the other side of the pipeline has been closed. I'll give a build with this a shot on my server, see how it flies, then merge this PR

@miniduikboot miniduikboot merged commit a93f71c into Impostor:master Jun 27, 2026
1 check passed
NikoCat233 added a commit to NikoCat233/Impostor.Hazel that referenced this pull request Jun 27, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants