Browse code

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 <flavio.crisciani@docker.com>

Flavio Crisciani authored on 2018/07/26 05:36:52
Showing 6 changed files
... ...
@@ -850,6 +850,7 @@ func parallelJoin(t *testing.T, rc libnetwork.Sandbox, ep libnetwork.Endpoint, t
850 850
 	err = ep.Join(sb)
851 851
 
852 852
 	runtime.LockOSThread()
853
+	defer runtime.UnlockOSThread()
853 854
 	if err != nil {
854 855
 		if _, ok := err.(types.ForbiddenError); !ok {
855 856
 			t.Fatalf("thread %d: %v", thrNumber, err)
... ...
@@ -867,6 +868,7 @@ func parallelLeave(t *testing.T, rc libnetwork.Sandbox, ep libnetwork.Endpoint,
867 867
 
868 868
 	err = ep.Leave(sb)
869 869
 	runtime.LockOSThread()
870
+	defer runtime.UnlockOSThread()
870 871
 	if err != nil {
871 872
 		if _, ok := err.(types.ForbiddenError); !ok {
872 873
 			t.Fatalf("thread %d: %v", thrNumber, err)
... ...
@@ -39,19 +39,6 @@ func Init() {
39 39
 	}
40 40
 }
41 41
 
42
-// SetNamespace sets the initial namespace handler
43
-func SetNamespace() error {
44
-	initOnce.Do(Init)
45
-	if err := netns.Set(initNs); err != nil {
46
-		linkInfo, linkErr := getLink()
47
-		if linkErr != nil {
48
-			linkInfo = linkErr.Error()
49
-		}
50
-		return fmt.Errorf("failed to set to initial namespace, %v, initns fd %d: %v", linkInfo, initNs, err)
51
-	}
52
-	return nil
53
-}
54
-
55 42
 // ParseHandlerInt transforms the namespace handler into an integer
56 43
 func ParseHandlerInt() int {
57 44
 	return int(getHandler())
... ...
@@ -394,13 +394,10 @@ func (n *networkNamespace) InvokeFunc(f func()) error {
394 394
 // InitOSContext initializes OS context while configuring network resources
395 395
 func InitOSContext() func() {
396 396
 	runtime.LockOSThread()
397
-	if err := ns.SetNamespace(); err != nil {
398
-		logrus.Error(err)
399
-	}
400 397
 	return runtime.UnlockOSThread
401 398
 }
402 399
 
403
-func nsInvoke(path string, prefunc func(nsFD int) error, postfunc func(callerFD int) error) error {
400
+func nsInvoke(path string, prefunc, postfunc func(int) error) error {
404 401
 	defer InitOSContext()()
405 402
 
406 403
 	newNs, err := netns.GetFromPath(path)
... ...
@@ -415,10 +412,17 @@ func nsInvoke(path string, prefunc func(nsFD int) error, postfunc func(callerFD
415 415
 		return fmt.Errorf("failed in prefunc: %v", err)
416 416
 	}
417 417
 
418
+	// save the current namespace (host namespace)
419
+	curNs, err := netns.Get()
420
+	if err != nil {
421
+		return err
422
+	}
423
+	defer curNs.Close()
418 424
 	if err = netns.Set(newNs); err != nil {
419 425
 		return err
420 426
 	}
421
-	defer ns.SetNamespace()
427
+	// will restore the previous namespace before unlocking the thread
428
+	defer netns.Set(curNs)
422 429
 
423 430
 	// Invoked after the namespace switch.
424 431
 	return postfunc(ns.ParseHandlerInt())
... ...
@@ -7,7 +7,6 @@ import (
7 7
 	"net"
8 8
 	"os"
9 9
 	"path/filepath"
10
-	"runtime"
11 10
 	"strings"
12 11
 	"syscall"
13 12
 	"testing"
... ...
@@ -199,7 +198,6 @@ func TestDisableIPv6DAD(t *testing.T) {
199 199
 	if err != nil {
200 200
 		t.Fatalf("Failed to create a new sandbox: %v", err)
201 201
 	}
202
-	runtime.LockOSThread()
203 202
 	defer s.Destroy()
204 203
 
205 204
 	n, ok := s.(*networkNamespace)
... ...
@@ -253,7 +251,6 @@ func TestSetInterfaceIP(t *testing.T) {
253 253
 	if err != nil {
254 254
 		t.Fatalf("Failed to create a new sandbox: %v", err)
255 255
 	}
256
-	runtime.LockOSThread()
257 256
 	defer s.Destroy()
258 257
 
259 258
 	n, ok := s.(*networkNamespace)
... ...
@@ -329,7 +326,6 @@ func TestLiveRestore(t *testing.T) {
329 329
 	if err != nil {
330 330
 		t.Fatalf("Failed to create a new sandbox: %v", err)
331 331
 	}
332
-	runtime.LockOSThread()
333 332
 	defer s.Destroy()
334 333
 
335 334
 	n, ok := s.(*networkNamespace)
... ...
@@ -2,7 +2,6 @@ package osl
2 2
 
3 3
 import (
4 4
 	"os"
5
-	"runtime"
6 5
 	"testing"
7 6
 
8 7
 	"github.com/docker/docker/pkg/reexec"
... ...
@@ -80,7 +79,6 @@ func TestSandboxCreateTwice(t *testing.T) {
80 80
 	if err != nil {
81 81
 		t.Fatalf("Failed to create a new sandbox: %v", err)
82 82
 	}
83
-	runtime.LockOSThread()
84 83
 
85 84
 	// Create another sandbox with the same key to see if we handle it
86 85
 	// gracefully.
... ...
@@ -88,7 +86,6 @@ func TestSandboxCreateTwice(t *testing.T) {
88 88
 	if err != nil {
89 89
 		t.Fatalf("Failed to create a new sandbox: %v", err)
90 90
 	}
91
-	runtime.LockOSThread()
92 91
 
93 92
 	err = s.Destroy()
94 93
 	if err != nil {
... ...
@@ -130,7 +127,6 @@ func TestAddRemoveInterface(t *testing.T) {
130 130
 	if err != nil {
131 131
 		t.Fatalf("Failed to create a new sandbox: %v", err)
132 132
 	}
133
-	runtime.LockOSThread()
134 133
 
135 134
 	if s.Key() != key {
136 135
 		t.Fatalf("s.Key() returned %s. Expected %s", s.Key(), key)
... ...
@@ -33,8 +33,6 @@ func SetupTestOSContext(t *testing.T) func() {
33 33
 	// sure to re-initialize initNs context
34 34
 	ns.Init()
35 35
 
36
-	runtime.LockOSThread()
37
-
38 36
 	return func() {
39 37
 		if err := syscall.Close(fd); err != nil {
40 38
 			t.Logf("Warning: netns closing failed (%v)", err)