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

SplitHTTP: Fix connection leaks and crashes #3710

Merged
merged 4 commits into from
Aug 22, 2024
Merged

Conversation

mmmray
Copy link
Collaborator

@mmmray mmmray commented Aug 20, 2024

There are a few bugs that are fixed here:

  1. (all http versions) When the client opens a connection and closes it without any traffic, the Close would lock forever and not appear on the wire at all
  2. (all http versions) When the client terminates the connection correctly, the server would not notice and keep the upload queue and related datastructures around until the proxied connection times out.
  3. (quic only) closing the connection might crash the client as well, Fix panic with this error #3699

Are fixed in the following way:

  • Problem 1 is addressed by removing the use of LazyReader. It turns out that while ok is being peeked, Close is entirely ignored
  • Problem 2 is addressed on the server by using the request's context to close the connection
  • Problem 3 is addressed by using the request's context to close the connection instead of Body.Close.

Fix XTLS#3699

I was not able to reproduce this issue at all, but the fix seems
straightforward.
@mmmray mmmray mentioned this pull request Aug 20, 2024
3 tasks
@kabo00os
Copy link

This problem occurs with burstObservatory
When several configs are added, sometimes this problem occurs

@mmmray mmmray marked this pull request as ready for review August 21, 2024 10:33
@mmmray mmmray marked this pull request as draft August 21, 2024 11:44
@mmmray mmmray changed the title SplitHTTP: Fix Read/Close race SplitHTTP: Fix connection leaks and crashes Aug 22, 2024
@mmmray mmmray marked this pull request as ready for review August 22, 2024 14:08
@mmmray mmmray merged commit 83eef6b into XTLS:main Aug 22, 2024
36 checks passed
@mmmray mmmray deleted the sh-readclose-race branch August 22, 2024 15:07
zxspirit pushed a commit to zxspirit/Xray-core that referenced this pull request Aug 30, 2024
leninalive pushed a commit to amnezia-vpn/amnezia-xray-core that referenced this pull request Oct 29, 2024
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.

panic with this error
2 participants