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.

Sign up for GitHub

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

Jump to bottom

No lock protection around client conn during reconnect #2335

Closed
dmcgowan opened this issue May 8, 2018 · 2 comments
Closed

No lock protection around client conn during reconnect #2335

dmcgowan opened this issue May 8, 2018 · 2 comments
Labels
exp/beginner help-wanted

Comments

@dmcgowan
Copy link
Member

dmcgowan commented May 8, 2018

After #2149 the client conn can be updated, however services are created using this client conn with no lock protection. We should protect this with a lock and document that these client instances of the services should not be kept around in scope for an extended period of time, since the connection may die.
@crosbymichael
Copy link
Member

What do you mean by client instances of services?

@dmcgowan
Copy link
Member Author

For example https://github.com/containerd/containerd/blob/v1.1.0/client.go#L455

No read lock when accessing and the created instance could end up using a connection different from the client it was created from after a reset.

@fraenkel fraenkel mentioned this issue Jul 14, 2018
Merged
@kadisi kadisi mentioned this issue Jul 20, 2018
Closed
fraenkel added a commit to fraenkel/containerd that referenced this issue Jul 24, 2018
@fraenkel
Use connection lock when creating services
9a4d1c5 
Fixes containerd#2335

Signed-off-by: Michael Fraenkel <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
exp/beginner help-wanted
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@dmcgowan @crosbymichael