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

Ensure iptables initialization only happens once #1676

Merged
merged 1 commit into from Mar 10, 2017

Conversation

Projects
None yet
2 participants
@maxvt
Contributor

maxvt commented Mar 8, 2017

I saw a rare race condition during the first few calls to iptables module
where some of them would reenter initCheck() after the first call
to it already changed iptablesPath, but before the rest of the function
completed (in particular the long execs into testing for availability
of --wait flag and determining iptables version), resulting in
failure of one or more of iptables calls that did not use --wait and
were concurrent.

To fix the problem, this change gathers all one-time initialization into a
single function under a sync.Once instead of using a global variable
as a "done initializing" flag before initialization is done. sync.Once
guarantees all concurrent calls will block until the first one completes.

In addition, it turns out that GetVersion(), called from initCheck(), used
Raw() which called back into initCheck() via raw(), which did not cause a
problem in the earlier implementation but deadlocked when initialization became
strict. This was changed to use a direct call, similar to initialization of
supportsXlock.

Signed-off-by: Max Timchenko max@maxvt.com

Ensure iptables initialization only happens once
I saw a rare race during the first few calls to iptables module
where some of them would reenter initCheck() after the first call
to it already changed iptablesPath, but before the rest of the function
completed (in particular the long execs into testing for availability
of --wait flag and determining iptables version), resulting in
failure of one or more of iptables calls that did not use --wait and
were concurrent.

To fix the problem, this change gathers all one-time initialization into a
single function under a sync.Once instead of using a global variable
as a "done initializing" flag before initialization is done. sync.Once
guarantees all concurrent calls will block until the first one completes.

In addition, it turns out that GetVersion(), called from initCheck(), used
Raw() which called back into initCheck() via raw(), which did not cause a
problem in the earlier implementation but deadlocked when initialization became
strict.  This was changed to use a direct call, similar to initialization of
supportsXlock.

Signed-off-by: Max Timchenko <max@maxvt.com>
@aboch

This comment has been minimized.

Show comment
Hide comment
@aboch

aboch Mar 8, 2017

Contributor

Thanks @maxvt

LGTM

Contributor

aboch commented Mar 8, 2017

Thanks @maxvt

LGTM

@aboch aboch merged commit 606a7aa into docker:master Mar 10, 2017

2 checks passed

ci/circleci Your tests passed on CircleCI!
Details
dco-signed All commits are signed

@maxvt maxvt deleted the maxvt:race-on-iptables-startup branch Mar 11, 2017

@aboch aboch referenced this pull request Mar 14, 2017

Merged

bump 17.04.0-rc1 #31811

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment