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

Default rejected-handler will leak buffers in raw stream HTTP servers #693

Open
DerGuteMoritz opened this issue Oct 10, 2023 · 1 comment

Comments

@DerGuteMoritz
Copy link
Collaborator

Problem

Given an HTTP server started with :raw-stream? true, when aleph.http.server/handle-request hits the RejectedExecutionException path, the default handler (i.e. when no custom rejected-handler was provided) will leak any byte buffers which are already present in the request's :body stream.

Possible solutions

  1. Drain the body channel and release all byte buffers still contained in it (does that cover all bases?)
  2. Document this fact (as suggested by @arnaudgeiser here)
  3. Require passing a custom rejected-handler when :raw-stream? true is passed
@KingMob
Copy link
Collaborator

KingMob commented Oct 16, 2023

I think if there's no user-supplied raw error handler, we should supply a default in raw-ring-handler. That default error handler could then close the stream, ms/consume all the ByteBufs and .release() them, as well as send a 503.

Hmmmm. We could also wrap a user-supplied error-handler with one which checks the refcnt and releases only if > 0. Theoretically this would be safe and helpful to users... But could there be a race if the user releases it, the ByteBuf goes back to the pool, and some non-netty thread grabs it? Maybe not.

I think you two know this area better than I, so I defer to your judgment.

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
@DerGuteMoritz @KingMob and others