Skip to content

some tweaks in both the splithttp and httpupgrade packages#3579

Closed
us254 wants to merge 2 commits intoXTLS:mainfrom
us254:main
Closed

some tweaks in both the splithttp and httpupgrade packages#3579
us254 wants to merge 2 commits intoXTLS:mainfrom
us254:main

Conversation

@us254
Copy link

@us254 us254 commented Jul 22, 2024

I've made some tweaks in both the splithttp and httpupgrade packages. Here's what's up:

In /splithttp/hub.go:

  1. Switched up the session reaper to use time.AfterFunc. Way cleaner.
  2. Fixed that lock issue in httpResponseBodyWriter.Write. No more deadlocks!
  3. Made ListenSH actually tell us when something's wrong.
  4. Added some defer cleanup in ListenSH. No more resource leaks.
  5. Threw in a dummy server for UDS. Keeps things stable.
  6. Properly handled ErrServerClosed. Smooth shutdowns, you know?

And in /httpupgrade/hub.go:

  1. Fixed that typo - innnerListener is now innerListener. Two n's, not three!
  2. Added a quick buffer clear after reading the HTTP request. Helps with WebSocket stuff later.
  3. Put the connection handling in its own goroutine. Should keep things moving if addConn gets slow.

@us254 us254 changed the title Fixed some issues in the HTTP upgrade server - take a look! Fixed some issues in the HTTP upgrade server Jul 22, 2024
@us254 us254 changed the title Fixed some issues in the HTTP upgrade server some tweaks in both the splithttp and httpupgrade packages Jul 22, 2024
@mmmray
Copy link
Contributor

mmmray commented Jul 22, 2024

I think this entire PR, but certainly the description is AI-generated. One can argue that some of these changes are useful (typo fixes, indeed there is something wrong with uds, see #3577), but the PR description even references some changes that are definitely not present in the PR. Most changes here are missing justification.

@mmmray mmmray closed this Jul 22, 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.

2 participants