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

pkg/ns README: expand on danger in ns switching in long-lived programs #331

Closed

Conversation

rosenhouse
Copy link
Contributor

@rosenhouse rosenhouse commented Nov 17, 2016

cc: @bboreham I realized there was another key point missing in the readme: that long-lived Go processes cannot switch namespaces safely, because even new goroutines may share threads with old goroutines.

Follow up to #330 and #262

@bboreham
Copy link
Contributor

(which calls Unshare from inside a new goroutine)

This isn't a necessary condition for trouble. Any time the Go runtime creates a new OS thread while the namespace was changed, you can expect some other goroutine to switch to that thread some time later.

@rosenhouse
Copy link
Contributor Author

Good point, I've re-written this section to focus on that point. That's the "checkmate" aspect of this. It is just totally outside the control of the Go programmer.

@dcbw
Copy link
Member

dcbw commented Nov 18, 2016

@rosenhouse can you explain the issue a bit more? I'm not quite seeing how a new goroutine that's assigned to the same OS thread as a Do() can execute and do anything until that Do() has reset the netns and exited... I'm very likely missing something :)

@bboreham
Copy link
Contributor

@dcbw It's not about a new goroutine assigned to the same OS thread; it's about a new OS thread created during the Do(), that is subsequently used for any goroutine. There's loads more information if you follow the links from #262

@rosenhouse
Copy link
Contributor Author

rosenhouse commented Nov 22, 2016

@dcbw @bboreham I put together a rough demo of the problem here.

The key part is here: spin up some goroutines on the main thread, and observe if their namespace is as expected.

Failures are frequent but random.

@squeed squeed added this to the v0.6.0 milestone Mar 1, 2017
@rosenhouse
Copy link
Contributor Author

Looks like Weave this issue, again: https://www.weave.works/blog/linux-namespaces-and-go-don-t-mix

@bboreham
Copy link
Contributor

bboreham commented Jun 3, 2017

@rosenhouse the ordering was: first Weave hit this issue, then Martynas found the cause, then Bryan came over here to point out it would hit CNI the same way 🙂, then Martynas wrote a blog post, then it sat in a publication queue for months.

(Bryan sat next to Martynas at the time)

@bboreham
Copy link
Contributor

bboreham commented Jun 3, 2017

This PR LGTM; @dcbw are you satisfied now?

rosenhouse added a commit to rosenhouse/plugins that referenced this pull request Jun 7, 2017
- redux of containernetworking/cni#331 since the
plugin/lib split
- added link to new blog post about this from Weave folks (thanks!)
@rosenhouse
Copy link
Contributor Author

Closing in favor of containernetworking/plugins#14

@rosenhouse rosenhouse closed this Jun 7, 2017
apreuavar6 added a commit to apreuavar6/cni-plugins that referenced this pull request Aug 11, 2024
- redux of containernetworking/cni#331 since the
plugin/lib split
- added link to new blog post about this from Weave folks (thanks!)
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.

4 participants