Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixed: Remove data race around default dialer #6

Merged
merged 29 commits into from
Jun 3, 2021

Conversation

ericrpowers
Copy link

Description

This PR is to sync up with latest upstream and pull in vulcand#200 to remove the data race within serveWebSocket where the websocket dialer was having concurrent read/writes.

juliens and others added 29 commits August 27, 2018 14:49
* fix: buffer error and go1.11.

* chore: add go1.11 in Travis.

* review: comment.
* feat: pass host header for Websocket.

* add tests.
* Flush when we have unannonced trailers

* Remove 1.8, not supported in golint anymore
* ensuring the CookieOption struct's fields are exported (vulcand#174)

* dep ensuring

* Updating failing test, I should pay more attention

* dep ensuring has caused problems, reverting. Although this is really strange that dep should do this
Go library change godoc to "pkg.go.dev"
* chore: update dependencies.

* feat: add more cookie options.

* chore: update travis configuration.
Update README.md Change godoc to "pkg.go.dev"
Co-authored-by: Tom Moulard <[email protected]>
Co-authored-by: M <[email protected]>
Co-authored-by: Sylvain Rabot <[email protected]>
This change makes it so that each forward gets its own dialer rather
then all sharing the global `websocket.DefaultDialer`. It fixes the flaky
`TestWebSocketNumGoRoutine` test and allows `WebsocketTLSClientConfig` to
set a different TLS config than the one used in the http `RoundTripper`,
the TLS config in the http `RoundTripper` will still be used as a
fallback if one wasn't set by the user.

Adds the new `optSetter` `WebsocketNetDialContext` to set a custom
DialContet for WebSocket use.

-  `go test -run=TestWebSocketNumGoRoutine -count=100 ./forward` now
   passes. Removed the skip directive.
-  Closes vulcand#199
-  Closes vulcand#125
@ericrpowers ericrpowers self-assigned this May 28, 2021
@ericrpowers ericrpowers marked this pull request as ready for review May 28, 2021 22:23
@ericrpowers ericrpowers merged commit 25abba7 into master Jun 3, 2021
@ericrpowers ericrpowers deleted the remove-data-race branch June 3, 2021 21:09
itsai220 added a commit that referenced this pull request Jul 13, 2021
* Use gorilla DialContext (vulcand#158)

* Handle Te header when http2 (vulcand#157)

* fix: buffer error and go1.11. (vulcand#159)

* fix: buffer error and go1.11.

* chore: add go1.11 in Travis.

* review: comment.

* fix: use internal logger. (vulcand#160)

* fix: body should never be nil (vulcand#162)

* Pass host header for Websocket. (vulcand#163)

* feat: pass host header for Websocket.

* add tests.

* fix: golint. (vulcand#165)

* Flush when we have unannonced trailers (vulcand#168)

* Flush when we have unannonced trailers

* Remove 1.8, not supported in golint anymore

* add missing callback on close of hijacked connections (vulcand#175)

* Allowing sticky session's affinity cookies to be created with more options such as HttpOnly or Secure (vulcand#174)

* Ensuring the CookieOption struct's fields are exported (vulcand#179)

* ensuring the CookieOption struct's fields are exported (vulcand#174)

* dep ensuring

* Updating failing test, I should pay more attention

* dep ensuring has caused problems, reverting. Although this is really strange that dep should do this

* chore: migrate to go module. (vulcand#182)

* Update README.md

Go library change godoc to "pkg.go.dev"

* feat: add more cookie options. (vulcand#195)

* chore: update dependencies.

* feat: add more cookie options.

* chore: update travis configuration.

* Add Fallback setter on CircuitBreaker (vulcand#201)

Signed-off-by: Sylvain Rabot <[email protected]>

* review.

* doc: buffer.go: s/request/response/ (vulcand#172)

* doc: typo fix /serveHTTP/serveWebSocket/s (vulcand#176)

* Make the MakeRateError delay field public so that custom RateErrHandlers can be more easily written (vulcand#206)

* fix typo (vulcand#212)

* Fix panics on ratelimit period (division by zero) (vulcand#213)

* Bump to go1.16 (vulcand#214)

* Add a mechanism to format the sticky cookie value (vulcand#216)

Co-authored-by: Tom Moulard <[email protected]>
Co-authored-by: M <[email protected]>
Co-authored-by: Sylvain Rabot <[email protected]>

* Do not use global websocket.DefaultDialer

This change makes it so that each forward gets its own dialer rather
then all sharing the global `websocket.DefaultDialer`. It fixes the flaky
`TestWebSocketNumGoRoutine` test and allows `WebsocketTLSClientConfig` to
set a different TLS config than the one used in the http `RoundTripper`,
the TLS config in the http `RoundTripper` will still be used as a
fallback if one wasn't set by the user.

Adds the new `optSetter` `WebsocketNetDialContext` to set a custom
DialContet for WebSocket use.

-  `go test -run=TestWebSocketNumGoRoutine -count=100 ./forward` now
   passes. Removed the skip directive.
-  Closes vulcand#199
-  Closes vulcand#125

* Revert un-skipping test

* Update: Cleanup local and remote merged changes

Co-authored-by: SALLEYRON Julien <[email protected]>
Co-authored-by: Ludovic Fernandez <[email protected]>
Co-authored-by: Michael <[email protected]>
Co-authored-by: ravilr <[email protected]>
Co-authored-by: Amir Keibi <[email protected]>
Co-authored-by: colynn.liu <[email protected]>
Co-authored-by: Sylvain Rabot <[email protected]>
Co-authored-by: Fernandez Ludovic <[email protected]>
Co-authored-by: colynn.liu <[email protected]>
Co-authored-by: Wes Turner <[email protected]>
Co-authored-by: Ben Yanke <[email protected]>
Co-authored-by: Peter C <[email protected]>
Co-authored-by: Iman Sahebi <[email protected]>
Co-authored-by: Douglas De Toni Machado <[email protected]>
Co-authored-by: Romain <[email protected]>
Co-authored-by: Jean-Baptiste Doumenjou <[email protected]>
Co-authored-by: Tom Moulard <[email protected]>
Co-authored-by: M <[email protected]>
Co-authored-by: Clifton Kaznocha <[email protected]>
Co-authored-by: Clifton Kaznocha <[email protected]>

Co-authored-by: Eric Powers <[email protected]>
Co-authored-by: SALLEYRON Julien <[email protected]>
Co-authored-by: Ludovic Fernandez <[email protected]>
Co-authored-by: Michael <[email protected]>
Co-authored-by: ravilr <[email protected]>
Co-authored-by: Amir Keibi <[email protected]>
Co-authored-by: colynn.liu <[email protected]>
Co-authored-by: Sylvain Rabot <[email protected]>
Co-authored-by: Fernandez Ludovic <[email protected]>
Co-authored-by: colynn.liu <[email protected]>
Co-authored-by: Wes Turner <[email protected]>
Co-authored-by: Ben Yanke <[email protected]>
Co-authored-by: Peter C <[email protected]>
Co-authored-by: Iman Sahebi <[email protected]>
Co-authored-by: Douglas De Toni Machado <[email protected]>
Co-authored-by: Romain <[email protected]>
Co-authored-by: Jean-Baptiste Doumenjou <[email protected]>
Co-authored-by: Tom Moulard <[email protected]>
Co-authored-by: M <[email protected]>
Co-authored-by: Clifton Kaznocha <[email protected]>
Co-authored-by: Clifton Kaznocha <[email protected]>
itsai220 added a commit that referenced this pull request Aug 4, 2021
* Use gorilla DialContext (vulcand#158)

* Handle Te header when http2 (vulcand#157)

* fix: buffer error and go1.11. (vulcand#159)

* fix: buffer error and go1.11.

* chore: add go1.11 in Travis.

* review: comment.

* fix: use internal logger. (vulcand#160)

* fix: body should never be nil (vulcand#162)

* Pass host header for Websocket. (vulcand#163)

* feat: pass host header for Websocket.

* add tests.

* fix: golint. (vulcand#165)

* Flush when we have unannonced trailers (vulcand#168)

* Flush when we have unannonced trailers

* Remove 1.8, not supported in golint anymore

* add missing callback on close of hijacked connections (vulcand#175)

* Allowing sticky session's affinity cookies to be created with more options such as HttpOnly or Secure (vulcand#174)

* Ensuring the CookieOption struct's fields are exported (vulcand#179)

* ensuring the CookieOption struct's fields are exported (vulcand#174)

* dep ensuring

* Updating failing test, I should pay more attention

* dep ensuring has caused problems, reverting. Although this is really strange that dep should do this

* chore: migrate to go module. (vulcand#182)

* Update README.md

Go library change godoc to "pkg.go.dev"

* feat: add more cookie options. (vulcand#195)

* chore: update dependencies.

* feat: add more cookie options.

* chore: update travis configuration.

* Add Fallback setter on CircuitBreaker (vulcand#201)

Signed-off-by: Sylvain Rabot <[email protected]>

* review.

* doc: buffer.go: s/request/response/ (vulcand#172)

* doc: typo fix /serveHTTP/serveWebSocket/s (vulcand#176)

* Make the MakeRateError delay field public so that custom RateErrHandlers can be more easily written (vulcand#206)

* fix typo (vulcand#212)

* Fix panics on ratelimit period (division by zero) (vulcand#213)

* Bump to go1.16 (vulcand#214)

* Add a mechanism to format the sticky cookie value (vulcand#216)

Co-authored-by: Tom Moulard <[email protected]>
Co-authored-by: M <[email protected]>
Co-authored-by: Sylvain Rabot <[email protected]>

* Do not use global websocket.DefaultDialer

This change makes it so that each forward gets its own dialer rather
then all sharing the global `websocket.DefaultDialer`. It fixes the flaky
`TestWebSocketNumGoRoutine` test and allows `WebsocketTLSClientConfig` to
set a different TLS config than the one used in the http `RoundTripper`,
the TLS config in the http `RoundTripper` will still be used as a
fallback if one wasn't set by the user.

Adds the new `optSetter` `WebsocketNetDialContext` to set a custom
DialContet for WebSocket use.

-  `go test -run=TestWebSocketNumGoRoutine -count=100 ./forward` now
   passes. Removed the skip directive.
-  Closes vulcand#199
-  Closes vulcand#125

* Revert un-skipping test

* Update: Cleanup local and remote merged changes

Co-authored-by: SALLEYRON Julien <[email protected]>
Co-authored-by: Ludovic Fernandez <[email protected]>
Co-authored-by: Michael <[email protected]>
Co-authored-by: ravilr <[email protected]>
Co-authored-by: Amir Keibi <[email protected]>
Co-authored-by: colynn.liu <[email protected]>
Co-authored-by: Sylvain Rabot <[email protected]>
Co-authored-by: Fernandez Ludovic <[email protected]>
Co-authored-by: colynn.liu <[email protected]>
Co-authored-by: Wes Turner <[email protected]>
Co-authored-by: Ben Yanke <[email protected]>
Co-authored-by: Peter C <[email protected]>
Co-authored-by: Iman Sahebi <[email protected]>
Co-authored-by: Douglas De Toni Machado <[email protected]>
Co-authored-by: Romain <[email protected]>
Co-authored-by: Jean-Baptiste Doumenjou <[email protected]>
Co-authored-by: Tom Moulard <[email protected]>
Co-authored-by: M <[email protected]>
Co-authored-by: Clifton Kaznocha <[email protected]>
Co-authored-by: Clifton Kaznocha <[email protected]>

Co-authored-by: Eric Powers <[email protected]>
Co-authored-by: SALLEYRON Julien <[email protected]>
Co-authored-by: Ludovic Fernandez <[email protected]>
Co-authored-by: Michael <[email protected]>
Co-authored-by: ravilr <[email protected]>
Co-authored-by: Amir Keibi <[email protected]>
Co-authored-by: colynn.liu <[email protected]>
Co-authored-by: Sylvain Rabot <[email protected]>
Co-authored-by: Fernandez Ludovic <[email protected]>
Co-authored-by: colynn.liu <[email protected]>
Co-authored-by: Wes Turner <[email protected]>
Co-authored-by: Ben Yanke <[email protected]>
Co-authored-by: Peter C <[email protected]>
Co-authored-by: Iman Sahebi <[email protected]>
Co-authored-by: Douglas De Toni Machado <[email protected]>
Co-authored-by: Romain <[email protected]>
Co-authored-by: Jean-Baptiste Doumenjou <[email protected]>
Co-authored-by: Tom Moulard <[email protected]>
Co-authored-by: M <[email protected]>
Co-authored-by: Clifton Kaznocha <[email protected]>
Co-authored-by: Clifton Kaznocha <[email protected]>
itsai220 added a commit that referenced this pull request Sep 2, 2021
* Fixed: Remove data race around default dialer (#6)

* Use gorilla DialContext (vulcand#158)

* Handle Te header when http2 (vulcand#157)

* fix: buffer error and go1.11. (vulcand#159)

* fix: buffer error and go1.11.

* chore: add go1.11 in Travis.

* review: comment.

* fix: use internal logger. (vulcand#160)

* fix: body should never be nil (vulcand#162)

* Pass host header for Websocket. (vulcand#163)

* feat: pass host header for Websocket.

* add tests.

* fix: golint. (vulcand#165)

* Flush when we have unannonced trailers (vulcand#168)

* Flush when we have unannonced trailers

* Remove 1.8, not supported in golint anymore

* add missing callback on close of hijacked connections (vulcand#175)

* Allowing sticky session's affinity cookies to be created with more options such as HttpOnly or Secure (vulcand#174)

* Ensuring the CookieOption struct's fields are exported (vulcand#179)

* ensuring the CookieOption struct's fields are exported (vulcand#174)

* dep ensuring

* Updating failing test, I should pay more attention

* dep ensuring has caused problems, reverting. Although this is really strange that dep should do this

* chore: migrate to go module. (vulcand#182)

* Update README.md

Go library change godoc to "pkg.go.dev"

* feat: add more cookie options. (vulcand#195)

* chore: update dependencies.

* feat: add more cookie options.

* chore: update travis configuration.

* Add Fallback setter on CircuitBreaker (vulcand#201)

Signed-off-by: Sylvain Rabot <[email protected]>

* review.

* doc: buffer.go: s/request/response/ (vulcand#172)

* doc: typo fix /serveHTTP/serveWebSocket/s (vulcand#176)

* Make the MakeRateError delay field public so that custom RateErrHandlers can be more easily written (vulcand#206)

* fix typo (vulcand#212)

* Fix panics on ratelimit period (division by zero) (vulcand#213)

* Bump to go1.16 (vulcand#214)

* Add a mechanism to format the sticky cookie value (vulcand#216)

Co-authored-by: Tom Moulard <[email protected]>
Co-authored-by: M <[email protected]>
Co-authored-by: Sylvain Rabot <[email protected]>

* Do not use global websocket.DefaultDialer

This change makes it so that each forward gets its own dialer rather
then all sharing the global `websocket.DefaultDialer`. It fixes the flaky
`TestWebSocketNumGoRoutine` test and allows `WebsocketTLSClientConfig` to
set a different TLS config than the one used in the http `RoundTripper`,
the TLS config in the http `RoundTripper` will still be used as a
fallback if one wasn't set by the user.

Adds the new `optSetter` `WebsocketNetDialContext` to set a custom
DialContet for WebSocket use.

-  `go test -run=TestWebSocketNumGoRoutine -count=100 ./forward` now
   passes. Removed the skip directive.
-  Closes vulcand#199
-  Closes vulcand#125

* Revert un-skipping test

* Update: Cleanup local and remote merged changes

Co-authored-by: SALLEYRON Julien <[email protected]>
Co-authored-by: Ludovic Fernandez <[email protected]>
Co-authored-by: Michael <[email protected]>
Co-authored-by: ravilr <[email protected]>
Co-authored-by: Amir Keibi <[email protected]>
Co-authored-by: colynn.liu <[email protected]>
Co-authored-by: Sylvain Rabot <[email protected]>
Co-authored-by: Fernandez Ludovic <[email protected]>
Co-authored-by: colynn.liu <[email protected]>
Co-authored-by: Wes Turner <[email protected]>
Co-authored-by: Ben Yanke <[email protected]>
Co-authored-by: Peter C <[email protected]>
Co-authored-by: Iman Sahebi <[email protected]>
Co-authored-by: Douglas De Toni Machado <[email protected]>
Co-authored-by: Romain <[email protected]>
Co-authored-by: Jean-Baptiste Doumenjou <[email protected]>
Co-authored-by: Tom Moulard <[email protected]>
Co-authored-by: M <[email protected]>
Co-authored-by: Clifton Kaznocha <[email protected]>
Co-authored-by: Clifton Kaznocha <[email protected]>

* fixed: forwarder did not set deadline for ws connections (#11)

* fixed: forwarder did not set deadline for ws connections

* remove debug prints

Co-authored-by: Primalmotion <[email protected]>

Co-authored-by: Eric Powers <[email protected]>
Co-authored-by: SALLEYRON Julien <[email protected]>
Co-authored-by: Ludovic Fernandez <[email protected]>
Co-authored-by: Michael <[email protected]>
Co-authored-by: ravilr <[email protected]>
Co-authored-by: Amir Keibi <[email protected]>
Co-authored-by: colynn.liu <[email protected]>
Co-authored-by: Sylvain Rabot <[email protected]>
Co-authored-by: Fernandez Ludovic <[email protected]>
Co-authored-by: colynn.liu <[email protected]>
Co-authored-by: Wes Turner <[email protected]>
Co-authored-by: Ben Yanke <[email protected]>
Co-authored-by: Peter C <[email protected]>
Co-authored-by: Iman Sahebi <[email protected]>
Co-authored-by: Douglas De Toni Machado <[email protected]>
Co-authored-by: Romain <[email protected]>
Co-authored-by: Jean-Baptiste Doumenjou <[email protected]>
Co-authored-by: Tom Moulard <[email protected]>
Co-authored-by: M <[email protected]>
Co-authored-by: Clifton Kaznocha <[email protected]>
Co-authored-by: Clifton Kaznocha <[email protected]>
Co-authored-by: primalmotion <[email protected]>
Co-authored-by: Primalmotion <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.