Merge pull request containerd#10059 from dcantah/ignore-eintr-helper

pkg/sys: Add helper to ignore eintr

core/metrics/cgroups/v1/oom.go

@@ -25,6 +25,7 @@ import (
 	"golang.org/x/sys/unix"
 
 	cgroups "github.com/containerd/cgroups/v3/cgroup1"
+	"github.com/containerd/containerd/v2/pkg/sys"
 	"github.com/containerd/log"
 	metrics "github.com/docker/go-metrics"
 	"github.com/prometheus/client_golang/prometheus"
@@ -110,16 +111,20 @@ func (o *oomCollector) Close() error {
 }
 
 func (o *oomCollector) start() {
-	var events [128]unix.EpollEvent
+	var (
+		n      int
+		err    error
+		events [128]unix.EpollEvent
+	)
 	for {
-		n, err := unix.EpollWait(o.fd, events[:], -1)
-		if err != nil {
-			if err == unix.EINTR {
-				continue
-			}
+		if err := sys.IgnoringEINTR(func() error {
+			n, err = unix.EpollWait(o.fd, events[:], -1)
+			return err
+		}); err != nil {
 			log.L.WithError(err).Error("cgroups: epoll wait failed, OOM notifications disabled")
 			return
 		}
+
 		for i := 0; i < n; i++ {
 			o.process(uintptr(events[i].Fd))
 		}

core/mount/mount_idmapped_linux.go

@@ -190,13 +190,9 @@ func getUsernsFD(uidMaps, gidMaps []syscall.SysProcIDMap) (_usernsFD *os.File, r
 }
 
 func pidfdWaitid(pidFD *os.File) error {
-	for {
-		err := unix.Waitid(unix.P_PIDFD, int(pidFD.Fd()), nil, unix.WEXITED, nil)
-		if err == unix.EINTR {
-			continue
-		}
-		return err
-	}
+	return sys.IgnoringEINTR(func() error {
+		return unix.Waitid(unix.P_PIDFD, int(pidFD.Fd()), nil, unix.WEXITED, nil)
+	})
 }
 
 var (

pkg/archive/link_freebsd.go

@@ -18,65 +18,17 @@ package archive
 
 import (
 	"os"
-	"syscall"
 
+	"github.com/containerd/containerd/v2/pkg/sys"
 	"golang.org/x/sys/unix"
 )
 
 func link(oldname, newname string) error {
-	e := ignoringEINTR(func() error {
+	e := sys.IgnoringEINTR(func() error {
 		return unix.Linkat(unix.AT_FDCWD, oldname, unix.AT_FDCWD, newname, 0)
 	})
 	if e != nil {
 		return &os.LinkError{Op: "link", Old: oldname, New: newname, Err: e}
 	}
 	return nil
 }
-
-// The following contents were copied from Go 1.18.2.
-// Use of this source code is governed by the following
-// BSD-style license:
-//
-// Copyright (c) 2009 The Go Authors. All rights reserved.
-//
-// Redistribution and use in source and binary forms, with or without
-// modification, are permitted provided that the following conditions are
-// met:
-//
-//   * Redistributions of source code must retain the above copyright
-// notice, this list of conditions and the following disclaimer.
-//   * Redistributions in binary form must reproduce the above
-// copyright notice, this list of conditions and the following disclaimer
-// in the documentation and/or other materials provided with the
-// distribution.
-//   * Neither the name of Google Inc. nor the names of its
-// contributors may be used to endorse or promote products derived from
-// this software without specific prior written permission.
-//
-// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
-// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
-// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
-// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
-// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
-// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
-// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
-// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
-// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
-// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
-// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
-
-// ignoringEINTR makes a function call and repeats it if it returns an
-// EINTR error. This appears to be required even though we install all
-// signal handlers with SA_RESTART: see #22838, #38033, #38836, #40846.
-// Also #20400 and #36644 are issues in which a signal handler is
-// installed without setting SA_RESTART. None of these are the common case,
-// but there are enough of them that it seems that we can't avoid
-// an EINTR loop.
-func ignoringEINTR(fn func() error) error {
-	for {
-		err := fn()
-		if err != syscall.EINTR {
-			return err
-		}
-	}
-}

pkg/oom/v1/v1.go

@@ -28,6 +28,7 @@ import (
 	"github.com/containerd/containerd/v2/core/runtime"
 	"github.com/containerd/containerd/v2/pkg/oom"
 	"github.com/containerd/containerd/v2/pkg/shim"
+	"github.com/containerd/containerd/v2/pkg/sys"
 	"github.com/containerd/log"
 	"golang.org/x/sys/unix"
 )
@@ -67,23 +68,31 @@ func (e *epoller) Close() error {
 
 // Run the epoll loop
 func (e *epoller) Run(ctx context.Context) {
-	var events [128]unix.EpollEvent
+	var (
+		n      int
+		err    error
+		events [128]unix.EpollEvent
+	)
 	for {
-		select {
-		case <-ctx.Done():
-			e.Close()
-			return
-		default:
-			n, err := unix.EpollWait(e.fd, events[:], -1)
-			if err != nil {
-				if err == unix.EINTR {
-					continue
-				}
-				log.G(ctx).WithError(err).Error("cgroups: epoll wait")
+		err = sys.IgnoringEINTR(func() error {
+			select {
+			case <-ctx.Done():
+				e.Close()
+				return ctx.Err()
+			default:
+				n, err = unix.EpollWait(e.fd, events[:], -1)
+				return err
 			}
-			for i := 0; i < n; i++ {
-				e.process(ctx, uintptr(events[i].Fd))
+		})
+		if err != nil {
+			if err == context.DeadlineExceeded || err == context.Canceled {
+				return
 			}
+			log.G(ctx).WithError(err).Error("cgroups: epoll wait")
+		}
+
+		for i := 0; i < n; i++ {
+			e.process(ctx, uintptr(events[i].Fd))
 		}
 	}
 }

pkg/sys/eintr_unix.go

@@ -0,0 +1,69 @@
+//go:build !windows
+
+/*
+   Copyright The containerd Authors.
+
+   Licensed under the Apache License, Version 2.0 (the "License");
+   you may not use this file except in compliance with the License.
+   You may obtain a copy of the License at
+
+       http://www.apache.org/licenses/LICENSE-2.0
+
+   Unless required by applicable law or agreed to in writing, software
+   distributed under the License is distributed on an "AS IS" BASIS,
+   WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+   See the License for the specific language governing permissions and
+   limitations under the License.
+*/
+
+package sys
+
+import "golang.org/x/sys/unix"
+
+// The following contents were copied from Go 1.18.2.
+// Use of this source code is governed by the following
+// BSD-style license:
+//
+// Copyright (c) 2009 The Go Authors. All rights reserved.
+//
+// Redistribution and use in source and binary forms, with or without
+// modification, are permitted provided that the following conditions are
+// met:
+//
+//   * Redistributions of source code must retain the above copyright
+// notice, this list of conditions and the following disclaimer.
+//   * Redistributions in binary form must reproduce the above
+// copyright notice, this list of conditions and the following disclaimer
+// in the documentation and/or other materials provided with the
+// distribution.
+//   * Neither the name of Google Inc. nor the names of its
+// contributors may be used to endorse or promote products derived from
+// this software without specific prior written permission.
+//
+// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+
+// IgnoringEINTR makes a function call and repeats it if it returns an
+// EINTR error. This appears to be required even though we install all
+// signal handlers with SA_RESTART: see #22838, #38033, #38836, #40846.
+// Also #20400 and #36644 are issues in which a signal handler is
+// installed without setting SA_RESTART. None of these are the common case,
+// but there are enough of them that it seems that we can't avoid
+// an EINTR loop.
+func IgnoringEINTR(fn func() error) error {
+	for {
+		err := fn()
+		if err != unix.EINTR {
+			return err
+		}
+	}
+}