Skip to content

Http request builder must not blindly wrap the request body in a NopCloser #678

Description

@oakad

As you may be aware, Go's HTTP client may retain the request Body reference after Do returns. The correct signal that Body can be reused is communicated via the Close method; that's why Go's http.Request.Body happens to be ReadCloser to start with.

When doing high throughput data processing it is imperative to reuse the large buffers used for data transfers. However, it seems, that with a present implementation, Body reference may still be in use by Go's HTTP client even after AWS SDK method returns. This makes it impossible to treat the return of the SDK method as buffer reuse boundary.

By itself it would not be an insurmountable problem - we could implement some nice Close methods on our buffer objects. However, these won't be called because smithy's Request object wraps away whatever custom Close possibly supplied by an user.

Moreover, whomever implemented the default Close method for smithy's HTTP Request objects was clearly aware that it may be called asynchronously from another goroutine and added a mutex in there. Unfortunately, the mutex on its own is not sufficient - in the absence of any API changes, smithy must guarantee, that AWS SDK methods cannot return before last Close was not called on the last rewound Body copy.

To summarize, smithy must ensure either that:

  1. User supplied Body reference is not accessible by any goroutine by the time enclosing SDK method returns (some sort of waitable object, channel or waitgroup inside the smithy implementation)
  2. Or, keep track of user supplied Close method on the Body and call that when last internal request Close is called.

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