Skip to content

Commit 1df7813

Browse files
authored
Merge pull request #212 from jepio/jepio/cgroup2-fix-inotify-leak
v2: Fix inotify fd leak when cgroup is deleted
2 parents f719f96 + a7d6888 commit 1df7813

File tree

4 files changed

+177
-52
lines changed

4 files changed

+177
-52
lines changed

go.mod

+3-2
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,8 @@ require (
1111
github.com/gogo/protobuf v1.3.2
1212
github.com/opencontainers/runtime-spec v1.0.2
1313
github.com/sirupsen/logrus v1.8.1
14-
github.com/stretchr/testify v1.6.1
14+
github.com/stretchr/testify v1.7.0
1515
github.com/urfave/cli v1.22.2
16-
golang.org/x/sys v0.0.0-20210124154548-22da62e12c0c
16+
go.uber.org/goleak v1.1.12
17+
golang.org/x/sys v0.0.0-20210510120138-977fb7262007
1718
)

go.sum

+22-4
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ github.com/google/go-cmp v0.5.4 h1:L8R9j+yAqZuZjsqh/z+F1NCffTKKLShY6zXTItVIZ8M=
2121
github.com/google/go-cmp v0.5.4/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE=
2222
github.com/kisielk/errcheck v1.5.0/go.mod h1:pFxgyoBC7bSaBwPgfKdkLd5X25qrDl4LWUI2bnpBCr8=
2323
github.com/kisielk/gotool v1.0.0/go.mod h1:XhKaO+MFFWcvkIS/tQcRk01m1F5IRFswLeQ+oQHNcck=
24+
github.com/kr/pretty v0.1.0/go.mod h1:dAy3ld7l9f0ibDNOQOHHMYYIIbhfbHSm3C4ZsoJORNo=
2425
github.com/kr/pretty v0.2.1 h1:Fmg33tUaq4/8ym9TJN1x7sLJnHVwhP33CNkpYV/7rwI=
2526
github.com/kr/pretty v0.2.1/go.mod h1:ipq/a2n7PKx3OHsz4KJII5eveXtPO4qwEXGdVfWzfnI=
2627
github.com/kr/pty v1.1.1/go.mod h1:pFQYn66WHrOpPYNljwOMqo10TkYh1fy3cYio2l3bCsQ=
@@ -38,43 +39,60 @@ github.com/sirupsen/logrus v1.8.1 h1:dJKuHgqk1NNQlqoA6BTlM1Wf9DOH3NBjQyu0h9+AZZE
3839
github.com/sirupsen/logrus v1.8.1/go.mod h1:yWOB1SBYBC5VeMP7gHvWumXLIWorT60ONWic61uBYv0=
3940
github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME=
4041
github.com/stretchr/testify v1.2.2/go.mod h1:a8OnRcib4nhh0OaRAV+Yts87kKdq0PP7pXfy6kDkUVs=
41-
github.com/stretchr/testify v1.6.1 h1:hDPOHmpOpP40lSULcqw7IrRb/u7w6RpDC9399XyoNd0=
42-
github.com/stretchr/testify v1.6.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg=
42+
github.com/stretchr/testify v1.7.0 h1:nwc3DEeHmmLAfoZucVR881uASk0Mfjw8xYJ99tb5CcY=
43+
github.com/stretchr/testify v1.7.0/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg=
4344
github.com/urfave/cli v1.22.2 h1:gsqYFH8bb9ekPA12kRo0hfjngWQjkJPlN9R0N78BoUo=
4445
github.com/urfave/cli v1.22.2/go.mod h1:Gos4lmkARVdJ6EkW0WaNv/tZAAMe9V7XWyB60NtXRu0=
4546
github.com/yuin/goldmark v1.1.27/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74=
4647
github.com/yuin/goldmark v1.2.1/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74=
48+
github.com/yuin/goldmark v1.3.5/go.mod h1:mwnBkeHKe2W/ZEtQ+71ViKU8L12m81fl3OWwC1Zlc8k=
49+
go.uber.org/goleak v1.1.12 h1:gZAh5/EyT/HQwlpkCy6wTpqfH9H8Lz8zbm3dZh+OyzA=
50+
go.uber.org/goleak v1.1.12/go.mod h1:cwTWslyiVhfpKIDGSZEM2HlOvcqm+tG4zioyIeLoqMQ=
4751
golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w=
4852
golang.org/x/crypto v0.0.0-20191011191535-87dc89f01550/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI=
4953
golang.org/x/crypto v0.0.0-20200622213623-75b288015ac9/go.mod h1:LzIPMQfyMNhhGPhUkYOs5KpL4U8rLKemX1yGLhDgUto=
54+
golang.org/x/lint v0.0.0-20190930215403-16217165b5de h1:5hukYrvBGR8/eNkX5mdUezrA6JiaEZDtJb9Ei+1LlBs=
55+
golang.org/x/lint v0.0.0-20190930215403-16217165b5de/go.mod h1:6SW0HCj/g11FgYtHlgUYUwCkIfeOF89ocIRzGO/8vkc=
5056
golang.org/x/mod v0.2.0/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA=
5157
golang.org/x/mod v0.3.0/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA=
58+
golang.org/x/mod v0.4.2/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA=
59+
golang.org/x/net v0.0.0-20190311183353-d8887717615a/go.mod h1:t9HGtf8HONx5eT2rtn7q6eTqICYqUVnKs3thJo3Qplg=
5260
golang.org/x/net v0.0.0-20190404232315-eb5bcb51f2a3/go.mod h1:t9HGtf8HONx5eT2rtn7q6eTqICYqUVnKs3thJo3Qplg=
5361
golang.org/x/net v0.0.0-20190620200207-3b0461eec859/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s=
5462
golang.org/x/net v0.0.0-20200226121028-0de0cce0169b/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s=
5563
golang.org/x/net v0.0.0-20201021035429-f5854403a974/go.mod h1:sp8m0HH+o8qH0wwXwYZr8TS3Oi6o0r6Gce1SSxlDquU=
64+
golang.org/x/net v0.0.0-20210405180319-a5a99cb37ef4/go.mod h1:p54w0d4576C0XHj96bSt6lcn1PtDYWL6XObtHCRCNQM=
5665
golang.org/x/sync v0.0.0-20190423024810-112230192c58/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
5766
golang.org/x/sync v0.0.0-20190911185100-cd5d95a43a6e/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
5867
golang.org/x/sync v0.0.0-20201020160332-67f06af15bc9/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
68+
golang.org/x/sync v0.0.0-20210220032951-036812b2e83c/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
5969
golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY=
6070
golang.org/x/sys v0.0.0-20190412213103-97732733099d/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
6171
golang.org/x/sys v0.0.0-20191026070338-33540a1f6037/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
6272
golang.org/x/sys v0.0.0-20200930185726-fdedc70b468f/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
63-
golang.org/x/sys v0.0.0-20210124154548-22da62e12c0c h1:VwygUrnw9jn88c4u8GD3rZQbqrP/tgas88tPUbBxQrk=
73+
golang.org/x/sys v0.0.0-20201119102817-f84b799fce68/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
6474
golang.org/x/sys v0.0.0-20210124154548-22da62e12c0c/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
75+
golang.org/x/sys v0.0.0-20210330210617-4fbd30eecc44/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
76+
golang.org/x/sys v0.0.0-20210510120138-977fb7262007 h1:gG67DSER+11cZvqIMb8S8bt0vZtiN6xWYARwirrOSfE=
77+
golang.org/x/sys v0.0.0-20210510120138-977fb7262007/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
78+
golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo=
6579
golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ=
6680
golang.org/x/text v0.3.3/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ=
6781
golang.org/x/tools v0.0.0-20180917221912-90fa682c2a6e/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ=
82+
golang.org/x/tools v0.0.0-20190311212946-11955173bddd/go.mod h1:LCzVGOaR6xXOjkQ3onu1FJEFr0SW1gC7cKk1uF8kGRs=
6883
golang.org/x/tools v0.0.0-20191119224855-298f0cb1881e/go.mod h1:b+2E5dAYhXwXZwtnZ6UAqBI28+e2cm9otk0dWdXHAEo=
6984
golang.org/x/tools v0.0.0-20200619180055-7c47624df98f/go.mod h1:EkVYQZoAsY45+roYkvgYkIh4xh/qjgUK9TdY2XT94GE=
7085
golang.org/x/tools v0.0.0-20210106214847-113979e3529a/go.mod h1:emZCQorbCU4vsT4fOWvOPXz4eW1wZW4PmDk9uLelYpA=
86+
golang.org/x/tools v0.1.5 h1:ouewzE6p+/VEB31YYnTbEJdi8pFqKp4P4n85vwo3DHA=
87+
golang.org/x/tools v0.1.5/go.mod h1:o0xws9oXOQQZyjljx8fwUC0k7L1pTE6eaCbjGeHmOkk=
7188
golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
7289
golang.org/x/xerrors v0.0.0-20191011141410-1b5146add898/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
7390
golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
7491
golang.org/x/xerrors v0.0.0-20200804184101-5ec99f83aff1 h1:go1bK/D/BFZV2I8cIQd1NKEZ+0owSTG1fDTci4IqFcE=
7592
golang.org/x/xerrors v0.0.0-20200804184101-5ec99f83aff1/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
76-
gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM=
7793
gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
94+
gopkg.in/check.v1 v1.0.0-20180628173108-788fd7840127 h1:qIbj1fsPNlZgppZ+VLlY7N33q108Sa+fhmuc+sWQYwY=
95+
gopkg.in/check.v1 v1.0.0-20180628173108-788fd7840127/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
7896
gopkg.in/yaml.v2 v2.2.2/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI=
7997
gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c h1:dUUwHk2QECo/6vqA44rthZ8ie2QXMNeKRTHCNY2nXvo=
8098
gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM=

v2/manager.go

+76-46
Original file line numberDiff line numberDiff line change
@@ -560,6 +560,22 @@ func (c *Manager) freeze(path string, state State) error {
560560
}
561561
}
562562

563+
func (c *Manager) isCgroupEmpty() bool {
564+
// In case of any error we return true so that we exit and don't leak resources
565+
out := make(map[string]interface{})
566+
if err := readKVStatsFile(c.path, "cgroup.events", out); err != nil {
567+
return true
568+
}
569+
if v, ok := out["populated"]; ok {
570+
populated, ok := v.(uint64)
571+
if !ok {
572+
return true
573+
}
574+
return populated == 0
575+
}
576+
return true
577+
}
578+
563579
// MemoryEventFD returns inotify file descriptor and 'memory.events' inotify watch descriptor
564580
func (c *Manager) MemoryEventFD() (int, uint32, error) {
565581
fpath := filepath.Join(c.path, "memory.events")
@@ -568,32 +584,72 @@ func (c *Manager) MemoryEventFD() (int, uint32, error) {
568584
return 0, 0, errors.New("failed to create inotify fd")
569585
}
570586
wd, err := syscall.InotifyAddWatch(fd, fpath, unix.IN_MODIFY)
571-
if wd < 0 {
587+
if err != nil {
588+
syscall.Close(fd)
589+
return 0, 0, fmt.Errorf("failed to add inotify watch for %q: %w", fpath, err)
590+
}
591+
// monitor to detect process exit/cgroup deletion
592+
evpath := filepath.Join(c.path, "cgroup.events")
593+
if _, err = syscall.InotifyAddWatch(fd, evpath, unix.IN_MODIFY); err != nil {
572594
syscall.Close(fd)
573-
return 0, 0, fmt.Errorf("failed to add inotify watch for %q", fpath)
595+
return 0, 0, fmt.Errorf("failed to add inotify watch for %q: %w", evpath, err)
574596
}
575597

576598
return fd, uint32(wd), nil
577599
}
578600

579601
func (c *Manager) EventChan() (<-chan Event, <-chan error) {
580602
ec := make(chan Event)
581-
errCh := make(chan error)
603+
errCh := make(chan error, 1)
582604
go c.waitForEvents(ec, errCh)
583605

584-
return ec, nil
606+
return ec, errCh
585607
}
586608

587-
func (c *Manager) waitForEvents(ec chan<- Event, errCh chan<- error) {
588-
fd, wd, err := c.MemoryEventFD()
609+
func parseMemoryEvents(out map[string]interface{}) (Event, error) {
610+
e := Event{}
611+
if v, ok := out["high"]; ok {
612+
e.High, ok = v.(uint64)
613+
if !ok {
614+
return Event{}, fmt.Errorf("cannot convert high to uint64: %+v", v)
615+
}
616+
}
617+
if v, ok := out["low"]; ok {
618+
e.Low, ok = v.(uint64)
619+
if !ok {
620+
return Event{}, fmt.Errorf("cannot convert low to uint64: %+v", v)
621+
}
622+
}
623+
if v, ok := out["max"]; ok {
624+
e.Max, ok = v.(uint64)
625+
if !ok {
626+
return Event{}, fmt.Errorf("cannot convert max to uint64: %+v", v)
627+
}
628+
}
629+
if v, ok := out["oom"]; ok {
630+
e.OOM, ok = v.(uint64)
631+
if !ok {
632+
return Event{}, fmt.Errorf("cannot convert oom to uint64: %+v", v)
633+
}
634+
}
635+
if v, ok := out["oom_kill"]; ok {
636+
e.OOMKill, ok = v.(uint64)
637+
if !ok {
638+
return Event{}, fmt.Errorf("cannot convert oom_kill to uint64: %+v", v)
639+
}
640+
}
641+
return e, nil
642+
}
589643

590-
defer syscall.InotifyRmWatch(fd, wd)
591-
defer syscall.Close(fd)
644+
func (c *Manager) waitForEvents(ec chan<- Event, errCh chan<- error) {
645+
defer close(errCh)
592646

647+
fd, _, err := c.MemoryEventFD()
593648
if err != nil {
594649
errCh <- err
595650
return
596651
}
652+
defer syscall.Close(fd)
597653

598654
for {
599655
buffer := make([]byte, syscall.SizeofInotifyEvent*10)
@@ -604,48 +660,22 @@ func (c *Manager) waitForEvents(ec chan<- Event, errCh chan<- error) {
604660
}
605661
if bytesRead >= syscall.SizeofInotifyEvent {
606662
out := make(map[string]interface{})
607-
if err := readKVStatsFile(c.path, "memory.events", out); err == nil {
608-
e := Event{}
609-
if v, ok := out["high"]; ok {
610-
e.High, ok = v.(uint64)
611-
if !ok {
612-
errCh <- fmt.Errorf("cannot convert high to uint64: %+v", v)
613-
return
614-
}
663+
if err := readKVStatsFile(c.path, "memory.events", out); err != nil {
664+
// When cgroup is deleted read may return -ENODEV instead of -ENOENT from open.
665+
if _, statErr := os.Lstat(filepath.Join(c.path, "memory.events")); !os.IsNotExist(statErr) {
666+
errCh <- err
615667
}
616-
if v, ok := out["low"]; ok {
617-
e.Low, ok = v.(uint64)
618-
if !ok {
619-
errCh <- fmt.Errorf("cannot convert low to uint64: %+v", v)
620-
return
621-
}
622-
}
623-
if v, ok := out["max"]; ok {
624-
e.Max, ok = v.(uint64)
625-
if !ok {
626-
errCh <- fmt.Errorf("cannot convert max to uint64: %+v", v)
627-
return
628-
}
629-
}
630-
if v, ok := out["oom"]; ok {
631-
e.OOM, ok = v.(uint64)
632-
if !ok {
633-
errCh <- fmt.Errorf("cannot convert oom to uint64: %+v", v)
634-
return
635-
}
636-
}
637-
if v, ok := out["oom_kill"]; ok {
638-
e.OOMKill, ok = v.(uint64)
639-
if !ok {
640-
errCh <- fmt.Errorf("cannot convert oom_kill to uint64: %+v", v)
641-
return
642-
}
643-
}
644-
ec <- e
645-
} else {
668+
return
669+
}
670+
e, err := parseMemoryEvents(out)
671+
if err != nil {
646672
errCh <- err
647673
return
648674
}
675+
ec <- e
676+
if c.isCgroupEmpty() {
677+
return
678+
}
649679
}
650680
}
651681
}

v2/manager_test.go

+76
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,76 @@
1+
/*
2+
Copyright The containerd Authors.
3+
4+
Licensed under the Apache License, Version 2.0 (the "License");
5+
you may not use this file except in compliance with the License.
6+
You may obtain a copy of the License at
7+
8+
http://www.apache.org/licenses/LICENSE-2.0
9+
10+
Unless required by applicable law or agreed to in writing, software
11+
distributed under the License is distributed on an "AS IS" BASIS,
12+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
See the License for the specific language governing permissions and
14+
limitations under the License.
15+
*/
16+
17+
package v2
18+
19+
import (
20+
"fmt"
21+
"os/exec"
22+
"testing"
23+
"time"
24+
25+
"go.uber.org/goleak"
26+
)
27+
28+
func TestEventChanCleanupOnCgroupRemoval(t *testing.T) {
29+
checkCgroupMode(t)
30+
31+
cmd := exec.Command("cat")
32+
stdin, err := cmd.StdinPipe()
33+
if err != nil {
34+
t.Fatalf("Failed to create cat process: %v", err)
35+
}
36+
if err := cmd.Start(); err != nil {
37+
t.Fatalf("Failed to start cat process: %v", err)
38+
}
39+
proc := cmd.Process
40+
if proc == nil {
41+
t.Fatal("Process is nil")
42+
}
43+
44+
group := fmt.Sprintf("testing-watcher-%d.scope", proc.Pid)
45+
c, err := NewSystemd("", group, proc.Pid, &Resources{})
46+
if err != nil {
47+
t.Fatalf("Failed to init new cgroup manager: %v", err)
48+
}
49+
50+
evCh, errCh := c.EventChan()
51+
52+
// give event goroutine a chance to start
53+
time.Sleep(500 * time.Millisecond)
54+
55+
if err := stdin.Close(); err != nil {
56+
t.Fatalf("Failed closing stdin: %v", err)
57+
}
58+
if err := cmd.Wait(); err != nil {
59+
t.Fatalf("Failed waiting for cmd: %v", err)
60+
}
61+
62+
done := false
63+
for !done {
64+
select {
65+
case <-evCh:
66+
case err := <-errCh:
67+
if err != nil {
68+
t.Fatalf("Unexpected error on error channel: %v", err)
69+
}
70+
done = true
71+
case <-time.After(5 * time.Second):
72+
t.Fatal("Timed out")
73+
}
74+
}
75+
goleak.VerifyNone(t)
76+
}

0 commit comments

Comments
 (0)