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

Fix race condition in GetCurrentNS #523

Merged
merged 1 commit into from
Aug 26, 2020
Merged

Conversation

tnqn
Copy link
Contributor

@tnqn tnqn commented Aug 21, 2020

In GetCurrentNS, If there is a context-switch between
getCurrentThreadNetNSPath and GetNS, another goroutine may execute in
the original thread and change its network namespace, then the original
goroutine would get the updated network namespace, which could lead to
unexpected behavior, especially when GetCurrentNS is used to get the
host network namespace in netNS.Do.

The added test has a chance to reproduce it with "-count=50".

The patch fixes it by locking the thread in GetCurrentNS.

Fixes #524

In GetCurrentNS, If there is a context-switch between
getCurrentThreadNetNSPath and GetNS, another goroutine may execute in
the original thread and change its network namespace, then the original
goroutine would get the updated network namespace, which could lead to
unexpected behavior, especially when GetCurrentNS is used to get the
host network namespace in netNS.Do.

The added test has a chance to reproduce it with "-count=50".

The patch fixes it by locking the thread in GetCurrentNS.

Signed-off-by: Quan Tian <[email protected]>
@dcbw
Copy link
Member

dcbw commented Aug 26, 2020

/lgtm

@squeed
Copy link
Member

squeed commented Aug 26, 2020

Agreed. test failures are windows

@squeed squeed merged commit dacb671 into containernetworking:master Aug 26, 2020
@tnqn tnqn deleted the ns-race branch August 26, 2020 15:47
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.

GetCurrentNS doesn't always return the "current" netns
3 participants