Skip to content

AHC crashes with HTTP2 when uploading a big body when channel & delegate live on separate EventLoops #784

New issue
New issue
Closed
#785
Closed
AHC crashes with HTTP2 when uploading a big body when channel & delegate live on separate EventLoops#784
#785
@weissi

Description

@weissi
weissi
opened on Nov 22, 2024

This is a pretty fun bug: In (at least) HTTP2 mode, when uploading a huge body, AHC behaves badly in various different ways.
The writes (in a recursive loop) are run in a recursive loop and they are freewheeling meaning there is no real backpressure.

Specifically in

async-http-client/Sources/AsyncHTTPClient/RequestBag.swift

Lines 187 to 188 in bdaa3b1

writer.writeRequestBodyPart(part, request: self, promise: promise)
return future
, we don't return the actual write promise but rather a pre-succeeded future. This won't stop until the state machine switches into a mode to "pause" sending the request body.

The problem is twofold:

  1. When delegate & channel are not co-located, the "pause" can arrive quite a bit too late because it's triggered from the channel's channelWritabilityChanged -> false which of course happens on the channel's EL. But the writes are triggered on the delegate's EL... (That's bad usually not fatal, we're just eating up some extra memory)
  2. Unfortunately, (in

    async-http-client/Sources/AsyncHTTPClient/HTTPHandler.swift

    Lines 58 to 69 in bdaa3b1

    func writeNextChunk(_ chunk: Bytes.SubSequence) -> EventLoopFuture<Void> {
    if let nextChunk = iterator.wrappedValue.next() {
    return self.write(.byteBuffer(ByteBuffer(bytes: chunk))).flatMap {
    writeNextChunk(nextChunk)
    }
    } else {
    return self.write(.byteBuffer(ByteBuffer(bytes: chunk)))
    }
    }
    return writeNextChunk(chunk)
    }
    ) we are actually triggering the writes recursively! That means it's actually impossible to get the pause signal until we're done. We'll just recurse on that pre-fulfilled future (which will immediately execute) and never enter the event loop again where a "please pause sending bytes" is waiting to execute. This is always fatal if we can't tolerate O(bodySize / 4MB) recursive stack frames. 5 GB always does the job.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions