socketry / async-http

MIT License
298 stars 45 forks source link

Improve handling of unconsumed request bodies at end of http1 requests #138

Closed zarqman closed 8 months ago

zarqman commented 9 months ago

This PR includes 2 related items:

  1. Fixes an old regression where request = nil should be conditional upon the request body, but was incorrectly the response body. Looking at the git history, the original behavior was ... unless request.body and was changed in b3cc9ce, seemingly accidentally during a refactor.

    I occasionally experience random hangs when running my test suite and believe this might be one cause of them.

  2. Changes the handling of unconsumed bodies from request.finish to request.each{}.

    finish buffers the request fully in memory as a Buffered, causing needless memory bloat in the ruby process. Using each skips the buffering, but is otherwise behaves the same.

Types of Changes

Contribution

zarqman commented 9 months ago

Revised to remove the previous attempt at request.close as further testing shows it might be causing a regression. Instead, using request.each{} which functions like the original call to request.finish except that it avoids buffering the request body into memory.

ioquatix commented 8 months ago

This seems reasonable to me, thanks for investigating the history.