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

network: Buffer partial reads for cancellation safety #3

Merged
merged 1 commit into from
Aug 1, 2023

Conversation

joeykraut
Copy link
Member

@joeykraut joeykraut commented Aug 1, 2023

Purpose

This PR adds buffers for partial reads in the network layer of the MpcFabric. This is necessary because tokio::select may cancel a read future, which drops the underlying partially read buffer of the next message without resetting the quic stream's cursor.

This means the next time a read future is created, it will begin from the middle of a message in the quic stream, causing an invalid read or blocking on data that will never be available.

The fix is to buffer partial reads in the QuicTwoPartyNet instance, which is maintained across cancelled futures.

Testing

  • Unit and integration tests pass
  • Replicated the issue with a large circuit in renegade::circuits and verified that this issue was fixed

@joeykraut joeykraut force-pushed the joey/network-cancellation-safety branch from e19282a to 084b165 Compare August 1, 2023 17:55
@joeykraut joeykraut marked this pull request as ready for review August 1, 2023 17:58
@joeykraut joeykraut requested a review from akirillo August 1, 2023 17:58
Copy link

@akirillo akirillo left a comment

Choose a reason for hiding this comment

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

lgtm, left a small q / suggestion

src/network.rs Show resolved Hide resolved
@joeykraut joeykraut force-pushed the joey/network-cancellation-safety branch from 084b165 to a55a636 Compare August 1, 2023 21:05
@joeykraut joeykraut merged commit 7515486 into master Aug 1, 2023
4 checks passed
@joeykraut joeykraut deleted the joey/network-cancellation-safety branch August 12, 2023 18:42
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