Fix race in RetryableTransport

[vbrown608]

There's a race in our RetryableTransport because the submit method writes to an http.Client referenced by a pointer.

uploadFile is called concurrently here, which causes concurrent calls to RetryableTransport.Submit.

open-api/go/porcelain/deploy.go

Line 372 in 29814e0

go n.uploadFile(ctx, d, file, observer, t, timeout, wg, sem, sharedErr)

I'm a bit uncertain about how the OpenApi runtime works. The RetryableTransport has an http.RoundTripper on it, but the Submit method also also takes an op with it's own Client. I'm not really sure which one is used when. Open to another solution if someone knows this package well.

This PR resolves the race by copying op.Client rather than setting a new Transport on it directly.

It also adds a test to call this method concurrently so that it can be picked up by the race detector. We've been getting panics in Buildbot and I think this may be the reason.

[netlify]

Deploy preview for open-api ready!

Built with commit c065ece

https://deploy-preview-230--open-api.netlify.app

[mraerino]

Good catch.

it seems like a hacky solution, but i guess in order to fix it properly the retryable transport would have to be rewritten to change the transport on a different level?

[vbrown608]

it seems like a hacky solution, but i guess in order to fix it properly the retryable transport would have to be rewritten to change the transport on a different level?

Yep!

[keiko713]

yay!! no more race! 🏇