Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MessageBuilder: header partial read #299

Closed
zii-dmg opened this issue Oct 23, 2024 · 3 comments
Closed

MessageBuilder: header partial read #299

zii-dmg opened this issue Oct 23, 2024 · 3 comments

Comments

@zii-dmg
Copy link

zii-dmg commented Oct 23, 2024

await stream.ReadAsync(headerBytes, 0, 24, token).ConfigureAwait(false);

There is no check for read count so header can be read partially. Then array has zero bytes at end and check for zeros generates exception "Null header data indicates peer disconnected". There should be while (readCount < 24) loop. I think I got partial reads on real network because logs contains "Null header data indicates peer disconnected".

Also read can return 0 count for gracefully closed socket.

Line:

await stream.ReadAsync(headerBuffer, 0, 1, token).ConfigureAwait(false);

Also should check for 0 count.

@sancheolz
Copy link

In addition: the client breaks the loop by IOException and calls HandleServerDisconnected in

_Events?.HandleServerDisconnected(this, new DisconnectionEventArgs(null, reason));

But at the same time, SendAsync is waiting on the server and it will not be released by CancelationToken.Cancel, because the client does not call Disconnect(), which should have sent the Shutdown packet. The TCP connection is active, and the client is in the _Connected = false state.
As a result, we have a hung thread on the sending side.

@sancheolz
Copy link

Related to #263.
I think the deadlock is happening there due to receiving an incomplete header. Setting NoDelay = true just hid the problem.

@jchristn
Copy link
Collaborator

Resolving in 6.0.8, uploading in a few moments.

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

No branches or pull requests

3 participants