From 94af1e5af201a87c651ef1a531bb19efdcd413e0 Mon Sep 17 00:00:00 2001 From: Flavio Crisciani Date: Wed, 25 Jul 2018 13:36:52 -0700 Subject: [PATCH] Adjust LockOSThread Go 1.10 fixed the problem related to thread and namespaces. Details: https://github.com/golang/go/commit/2595fe7fb6f272f9204ca3ef0b0c55e66fb8d90f In few words there is no more the possibility to have a go routine running on a thread that is another namespace. In this commit some cleanup is done and the method SetNamespace is being removed. This will save tons of setns syscall, that were happening way too frequently possibily to make sure that each operation was being done in the host namespace. I suspect that also all the drivers not running in a different namespace would be able to drop also the lock of the OS Thread but will address it in a different commit Removed useless LockOSThreads around Signed-off-by: Flavio Crisciani --- libnetwork/libnetwork_linux_test.go | 2 ++ libnetwork/ns/init_linux.go | 13 ------------- libnetwork/osl/namespace_linux.go | 14 +++++++++----- libnetwork/osl/sandbox_linux_test.go | 4 ---- libnetwork/osl/sandbox_test.go | 4 ---- libnetwork/testutils/context_unix.go | 2 -- 6 files changed, 11 insertions(+), 28 deletions(-) diff --git a/libnetwork/libnetwork_linux_test.go b/libnetwork/libnetwork_linux_test.go index 9c6b3b1d1c..4342aff05a 100644 --- a/libnetwork/libnetwork_linux_test.go +++ b/libnetwork/libnetwork_linux_test.go @@ -850,6 +850,7 @@ func parallelJoin(t *testing.T, rc libnetwork.Sandbox, ep libnetwork.Endpoint, t err = ep.Join(sb) runtime.LockOSThread() + defer runtime.UnlockOSThread() if err != nil { if _, ok := err.(types.ForbiddenError); !ok { t.Fatalf("thread %d: %v", thrNumber, err) @@ -867,6 +868,7 @@ func parallelLeave(t *testing.T, rc libnetwork.Sandbox, ep libnetwork.Endpoint, err = ep.Leave(sb) runtime.LockOSThread() + defer runtime.UnlockOSThread() if err != nil { if _, ok := err.(types.ForbiddenError); !ok { t.Fatalf("thread %d: %v", thrNumber, err) diff --git a/libnetwork/ns/init_linux.go b/libnetwork/ns/init_linux.go index 567a6242ac..05e34d83db 100644 --- a/libnetwork/ns/init_linux.go +++ b/libnetwork/ns/init_linux.go @@ -39,19 +39,6 @@ func Init() { } } -// SetNamespace sets the initial namespace handler -func SetNamespace() error { - initOnce.Do(Init) - if err := netns.Set(initNs); err != nil { - linkInfo, linkErr := getLink() - if linkErr != nil { - linkInfo = linkErr.Error() - } - return fmt.Errorf("failed to set to initial namespace, %v, initns fd %d: %v", linkInfo, initNs, err) - } - return nil -} - // ParseHandlerInt transforms the namespace handler into an integer func ParseHandlerInt() int { return int(getHandler()) diff --git a/libnetwork/osl/namespace_linux.go b/libnetwork/osl/namespace_linux.go index 45c46852fc..31f296d9bd 100644 --- a/libnetwork/osl/namespace_linux.go +++ b/libnetwork/osl/namespace_linux.go @@ -394,13 +394,10 @@ func (n *networkNamespace) InvokeFunc(f func()) error { // InitOSContext initializes OS context while configuring network resources func InitOSContext() func() { runtime.LockOSThread() - if err := ns.SetNamespace(); err != nil { - logrus.Error(err) - } return runtime.UnlockOSThread } -func nsInvoke(path string, prefunc func(nsFD int) error, postfunc func(callerFD int) error) error { +func nsInvoke(path string, prefunc, postfunc func(int) error) error { defer InitOSContext()() newNs, err := netns.GetFromPath(path) @@ -415,10 +412,17 @@ func nsInvoke(path string, prefunc func(nsFD int) error, postfunc func(callerFD return fmt.Errorf("failed in prefunc: %v", err) } + // save the current namespace (host namespace) + curNs, err := netns.Get() + if err != nil { + return err + } + defer curNs.Close() if err = netns.Set(newNs); err != nil { return err } - defer ns.SetNamespace() + // will restore the previous namespace before unlocking the thread + defer netns.Set(curNs) // Invoked after the namespace switch. return postfunc(ns.ParseHandlerInt()) diff --git a/libnetwork/osl/sandbox_linux_test.go b/libnetwork/osl/sandbox_linux_test.go index 2e82c9b8c3..74053d7108 100644 --- a/libnetwork/osl/sandbox_linux_test.go +++ b/libnetwork/osl/sandbox_linux_test.go @@ -7,7 +7,6 @@ import ( "net" "os" "path/filepath" - "runtime" "strings" "syscall" "testing" @@ -199,7 +198,6 @@ func TestDisableIPv6DAD(t *testing.T) { if err != nil { t.Fatalf("Failed to create a new sandbox: %v", err) } - runtime.LockOSThread() defer s.Destroy() n, ok := s.(*networkNamespace) @@ -253,7 +251,6 @@ func TestSetInterfaceIP(t *testing.T) { if err != nil { t.Fatalf("Failed to create a new sandbox: %v", err) } - runtime.LockOSThread() defer s.Destroy() n, ok := s.(*networkNamespace) @@ -329,7 +326,6 @@ func TestLiveRestore(t *testing.T) { if err != nil { t.Fatalf("Failed to create a new sandbox: %v", err) } - runtime.LockOSThread() defer s.Destroy() n, ok := s.(*networkNamespace) diff --git a/libnetwork/osl/sandbox_test.go b/libnetwork/osl/sandbox_test.go index 2f89fc3a10..44a0226caf 100644 --- a/libnetwork/osl/sandbox_test.go +++ b/libnetwork/osl/sandbox_test.go @@ -2,7 +2,6 @@ package osl import ( "os" - "runtime" "testing" "github.com/docker/docker/pkg/reexec" @@ -80,7 +79,6 @@ func TestSandboxCreateTwice(t *testing.T) { if err != nil { t.Fatalf("Failed to create a new sandbox: %v", err) } - runtime.LockOSThread() // Create another sandbox with the same key to see if we handle it // gracefully. @@ -88,7 +86,6 @@ func TestSandboxCreateTwice(t *testing.T) { if err != nil { t.Fatalf("Failed to create a new sandbox: %v", err) } - runtime.LockOSThread() err = s.Destroy() if err != nil { @@ -130,7 +127,6 @@ func TestAddRemoveInterface(t *testing.T) { if err != nil { t.Fatalf("Failed to create a new sandbox: %v", err) } - runtime.LockOSThread() if s.Key() != key { t.Fatalf("s.Key() returned %s. Expected %s", s.Key(), key) diff --git a/libnetwork/testutils/context_unix.go b/libnetwork/testutils/context_unix.go index 17ce4fef95..5908c05be6 100644 --- a/libnetwork/testutils/context_unix.go +++ b/libnetwork/testutils/context_unix.go @@ -33,8 +33,6 @@ func SetupTestOSContext(t *testing.T) func() { // sure to re-initialize initNs context ns.Init() - runtime.LockOSThread() - return func() { if err := syscall.Close(fd); err != nil { t.Logf("Warning: netns closing failed (%v)", err)