client: ensure requests are restarted in a race-free way.

[mhilton]

HTTP clients can return whilst they are still reading a request
body. Ensure that any request with a body is not restarted until the
previous request has closed it.

[rogpeppe]

LGTM modulo a couple of thoughts and suggestions, thanks.

[rogpeppe]

I know it did this before too, but side-effecting the contents of requestData is really quite icky. A caller might be reasonably entitled to assume that the ReqReader they put into the RequestData is the same after the call as before.

[rogpeppe]

For some readers, GetBody is populated automatically; could use that if it does it. shrug

[rogpeppe]

It would be nice to have a high level description of what this type does.

[rogpeppe]

Much as I appreciate a nice bit of slice twiddling, I think it might be possible to avoid it in this case. How about something like this?

type bufferingReader struct {
	mu  sync.Mutex
	buf bytes.Buffer
	r   io.Reader
}

func (r *bufferingReader) getReqReader() (io.ReadCloser, error) {
	r.mu.Lock()
	return &unlockingReadCloser{
		mu: &r.mu,
		Reader: io.MultiReader(
			bytes.NewReader(r.buf.Bytes()),
			io.TeeReader(r.r, &r.buf),
		),
	}, nil
}

type seekingReader struct {
	mu sync.Mutex
	r  io.ReadSeeker
}

func (r *seekingReader) getReqReader() (io.ReadCloser, error) {
	r.mu.Lock()
	if _, err := r.r.Seek(0, 0); err != nil {
		r.mu.Unlock()
		return nil, err
	}
	return &unlockingReadCloser{
		mu:     &r.mu,
		Reader: r.r,
	}, nil
}

type unlockingReadCloser struct {
	mu *sync.Mutex
	io.Reader
}

func (c *unlockingReadCloser) Close() error {
	if c.mu != nil {
		c.mu.Unlock()
		c.mu = nil
	}
	return nil
}

[rogpeppe]

Looks great, but maybe a final tweak might be good?

[rogpeppe]

This means that we flatly refuse to process any requests that are larger than the requested size. I thought we could do better than that, because it's quite possible that we will receive the error before all of maxBufSize bytes have been read.

The way I was previously envisaging can succeed in that case, I think.

[mhilton]

This is still better than the previous behavior which wouldn't even try the first time if it thought it was too long.

[rogpeppe]

This is quite long. Maybe put the various readers on different lines?
A comment would probably also be appropriate.