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

Use timeouts in BackendSession Recv method to fix leaking goroutine #65

Merged
merged 2 commits into from
Jul 17, 2020

Conversation

ghjm
Copy link
Contributor

@ghjm ghjm commented Jul 17, 2020

The goroutine leak in issue #62 is caused by BackendSession.Recv() being a blocking call, so the protoReader goroutine never exits. This PR adds a timeout value to Recv() so that protoReader has a chance to periodically check if the context is done.

The timeout implementation in the websockets backend cannot use SetReadDeadLine like the other backends do, because of gorilla/websocket#474. After a timeout error occurs, the websocket connection is no longer usable and must be closed. So instead, we start a goroutine called recvChannelizer which pushes messages to a channel, and exits after any error. The websockets backend Recv() message then provides "soft" timeouts using a select on this channel or the timeout.

@ghjm ghjm requested a review from squidboylan July 17, 2020 01:22
Copy link
Contributor

@squidboylan squidboylan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

feels like there should be a more event/context based way to do this but it fixes my test so im happy with it

@ghjm ghjm merged commit 18c7cec into ansible:devel Jul 17, 2020
@ghjm ghjm deleted the goroutine_leak branch July 17, 2020 14:42
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.

3 participants