Fix GoLand analyzer errors under runsc/...

PiperOrigin-RevId: 384344990

7 files changed, 56 insertions, 27 deletions

diff --git a/runsc/container/console_test.go b/runsc/container/console_test.go
index 79b056fce..8c65b9cd4 100644
--- a/runsc/container/console_test.go
+++ b/runsc/container/console_test.go
@@ -308,7 +308,9 @@ func TestJobControlSignalExec(t *testing.T) {
 	}
 
 	// Execute sleep.
-	ptyMaster.Write([]byte("sleep 100\n"))
+	if _, err := ptyMaster.Write([]byte("sleep 100\n")); err != nil {
+		t.Fatalf("ptyMaster.Write: %v", err)
+	}
 
 	// Wait for it to start. Sleep's PPID is bash's PID.
 	expectedPL = append(expectedPL, newProcessBuilder().PID(3).PPID(2).Cmd("sleep").Process())
@@ -411,7 +413,9 @@ func TestJobControlSignalRootContainer(t *testing.T) {
 	// which makes this a suitable Reader for WaitUntilRead.
 	ptyBuf := newBlockingBuffer()
 	tee := io.TeeReader(ptyMaster, ptyBuf)
-	go io.Copy(os.Stderr, tee)
+	go func() {
+		_, _ = io.Copy(os.Stderr, tee)
+	}()
 
 	// Start the container.
 	if err := c.Start(conf); err != nil {
@@ -444,7 +448,9 @@ func TestJobControlSignalRootContainer(t *testing.T) {
 	}
 
 	// Execute sleep via the terminal.
-	ptyMaster.Write([]byte("sleep 100\n"))
+	if _, err := ptyMaster.Write([]byte("sleep 100\n")); err != nil {
+		t.Fatalf("ptyMaster.Write(): %v", err)
+	}
 
 	// Wait for sleep to start.
 	expectedPL = append(expectedPL, newProcessBuilder().PID(2).PPID(1).Cmd("sleep").Process())
@@ -563,13 +569,15 @@ func TestMultiContainerTerminal(t *testing.T) {
 				// file. Writes after a certain point will block unless we drain the
 				// PTY, so we must continually copy from it.
 				//
-				// We log the output to stderr for debugabilitly, and also to a buffer,
+				// We log the output to stderr for debuggability, and also to a buffer,
 				// since we wait on particular output from bash below. We use a custom
 				// blockingBuffer which is thread-safe and also blocks on Read calls,
 				// which makes this a suitable Reader for WaitUntilRead.
 				ptyBuf := newBlockingBuffer()
 				tee := io.TeeReader(tc.master, ptyBuf)
-				go io.Copy(os.Stderr, tee)
+				go func() {
+					_, _ = io.Copy(os.Stderr, tee)
+				}()
 
 				// Wait for bash to start.
 				expectedPL := []*control.Process{
@@ -581,7 +589,9 @@ func TestMultiContainerTerminal(t *testing.T) {
 
 				// Execute echo command and check that it was executed correctly. Use
 				// a variable to ensure it's not matching against command echo.
-				tc.master.Write([]byte("echo foo-${PWD}-123\n"))
+				if _, err := tc.master.Write([]byte("echo foo-${PWD}-123\n")); err != nil {
+					t.Fatalf("master.Write(): %v", err)
+				}
 				if err := testutil.WaitUntilRead(ptyBuf, "foo-/-123", 5*time.Second); err != nil {
 					t.Fatalf("echo didn't execute: %v", err)
 				}
diff --git a/runsc/container/container.go b/runsc/container/container.go
index 0820edaec..b789bc7da 100644
--- a/runsc/container/container.go
+++ b/runsc/container/container.go
@@ -208,7 +208,7 @@ func New(conf *config.Config, args Args) (*Container, error) {
 	if err := c.Saver.lockForNew(); err != nil {
 		return nil, err
 	}
-	defer c.Saver.unlock()
+	defer c.Saver.unlockOrDie()
 
 	// If the metadata annotations indicate that this container should be started
 	// in an existing sandbox, we must do so. These are the possible metadata
@@ -340,7 +340,7 @@ func (c *Container) Start(conf *config.Config) error {
 	if err := c.Saver.lock(); err != nil {
 		return err
 	}
-	unlock := cleanup.Make(func() { c.Saver.unlock() })
+	unlock := cleanup.Make(c.Saver.unlockOrDie)
 	defer unlock.Clean()
 
 	if err := c.requireStatus("start", Created); err != nil {
@@ -426,7 +426,7 @@ func (c *Container) Restore(spec *specs.Spec, conf *config.Config, restoreFile s
 	if err := c.Saver.lock(); err != nil {
 		return err
 	}
-	defer c.Saver.unlock()
+	defer c.Saver.unlockOrDie()
 
 	if err := c.requireStatus("restore", Created); err != nil {
 		return err
@@ -614,7 +614,7 @@ func (c *Container) Pause() error {
 	if err := c.Saver.lock(); err != nil {
 		return err
 	}
-	defer c.Saver.unlock()
+	defer c.Saver.unlockOrDie()
 
 	if c.Status != Created && c.Status != Running {
 		return fmt.Errorf("cannot pause container %q in state %v", c.ID, c.Status)
@@ -634,7 +634,7 @@ func (c *Container) Resume() error {
 	if err := c.Saver.lock(); err != nil {
 		return err
 	}
-	defer c.Saver.unlock()
+	defer c.Saver.unlockOrDie()
 
 	if c.Status != Paused {
 		return fmt.Errorf("cannot resume container %q in state %v", c.ID, c.Status)
@@ -675,8 +675,8 @@ func (c *Container) Destroy() error {
 		return err
 	}
 	defer func() {
-		c.Saver.unlock()
-		c.Saver.close()
+		c.Saver.unlockOrDie()
+		_ = c.Saver.close()
 	}()
 
 	// Stored for later use as stop() sets c.Sandbox to nil.
@@ -1020,10 +1020,10 @@ func runInCgroup(cg *cgroup.Cgroup, fn func() error) error {
 		return fn()
 	}
 	restore, err := cg.Join()
-	defer restore()
 	if err != nil {
 		return err
 	}
+	defer restore()
 	return fn()
 }
 
diff --git a/runsc/container/container_test.go b/runsc/container/container_test.go
index 249324c5a..7360eae35 100644
--- a/runsc/container/container_test.go
+++ b/runsc/container/container_test.go
@@ -53,7 +53,10 @@ func TestMain(m *testing.M) {
 	if err := testutil.ConfigureExePath(); err != nil {
 		panic(err.Error())
 	}
-	specutils.MaybeRunAsRoot()
+	if err := specutils.MaybeRunAsRoot(); err != nil {
+		fmt.Fprintf(os.Stderr, "Error running as root: %v", err)
+		os.Exit(123)
+	}
 	os.Exit(m.Run())
 }
 
@@ -523,9 +526,11 @@ func TestLifecycle(t *testing.T) {
 				ws, err := c.Wait()
 				if err != nil {
 					ch <- err
+					return
 				}
 				if got, want := ws.Signal(), unix.SIGTERM; got != want {
 					ch <- fmt.Errorf("got signal %v, want %v", got, want)
+					return
 				}
 				ch <- nil
 			}()
@@ -1525,7 +1530,9 @@ func TestCapabilities(t *testing.T) {
 			defer os.Remove(exePath)
 
 			// Need to traverse the intermediate directory.
-			os.Chmod(rootDir, 0755)
+			if err := os.Chmod(rootDir, 0755); err != nil {
+				t.Fatal(err)
+			}
 
 			execArgs := &control.ExecArgs{
 				Filename:         exePath,
@@ -2153,7 +2160,7 @@ func doDestroyStartingTest(t *testing.T, vfs2 bool) {
 		go func() {
 			defer wg.Done()
 			// Ignore failures, start can fail if destroy runs first.
-			startCont.Start(conf)
+			_ = startCont.Start(conf)
 		}()
 
 		wg.Add(1)
diff --git a/runsc/container/hook.go b/runsc/container/hook.go
index 901607aee..ce1c9e1de 100644
--- a/runsc/container/hook.go
+++ b/runsc/container/hook.go
@@ -101,8 +101,8 @@ func executeHook(h specs.Hook, s specs.State) error {
 			return fmt.Errorf("failure executing hook %q, err: %v\nstdout: %s\nstderr: %s", h.Path, err, stdout.String(), stderr.String())
 		}
 	case <-timer:
-		cmd.Process.Kill()
-		cmd.Wait()
+		_ = cmd.Process.Kill()
+		_ = cmd.Wait()
 		return fmt.Errorf("timeout executing hook %q\nstdout: %s\nstderr: %s", h.Path, stdout.String(), stderr.String())
 	}
 
diff --git a/runsc/container/multi_container_test.go b/runsc/container/multi_container_test.go
index 0dbe1e323..58ae18232 100644
--- a/runsc/container/multi_container_test.go
+++ b/runsc/container/multi_container_test.go
@@ -894,7 +894,9 @@ func TestMultiContainerKillAll(t *testing.T) {
 		if tc.killContainer {
 			// First kill the init process to make the container be stopped with
 			// processes still running inside.
-			containers[1].SignalContainer(unix.SIGKILL, false)
+			if err := containers[1].SignalContainer(unix.SIGKILL, false); err != nil {
+				t.Fatalf("SignalContainer(): %v", err)
+			}
 			op := func() error {
 				c, err := Load(conf.RootDir, FullID{ContainerID: ids[1]}, LoadOpts{})
 				if err != nil {
@@ -912,7 +914,7 @@ func TestMultiContainerKillAll(t *testing.T) {
 
 		c, err := Load(conf.RootDir, FullID{ContainerID: ids[1]}, LoadOpts{})
 		if err != nil {
-			t.Fatalf("failed to load child container %q: %v", c.ID, err)
+			t.Fatalf("failed to load child container %q: %v", ids[1], err)
 		}
 		// Kill'Em All
 		if err := c.SignalContainer(unix.SIGKILL, true); err != nil {
@@ -1040,7 +1042,8 @@ func TestMultiContainerDestroyStarting(t *testing.T) {
 		wg.Add(1)
 		go func() {
 			defer wg.Done()
-			startCont.Start(conf) // ignore failures, start can fail if destroy runs first.
+			// Ignore failures, start can fail if destroy runs first.
+			_ = startCont.Start(conf)
 		}()
 
 		wg.Add(1)
@@ -1980,7 +1983,7 @@ func TestMultiContainerEvent(t *testing.T) {
 
 		if busyUsage <= sleepUsage {
 			t.Logf("Busy container usage lower than sleep (busy: %d, sleep: %d), retrying...", busyUsage, sleepUsage)
-			return fmt.Errorf("Busy container should have higher usage than sleep, busy: %d, sleep: %d", busyUsage, sleepUsage)
+			return fmt.Errorf("busy container should have higher usage than sleep, busy: %d, sleep: %d", busyUsage, sleepUsage)
 		}
 		return nil
 	}
diff --git a/runsc/container/shared_volume_test.go b/runsc/container/shared_volume_test.go
index cb5bffb89..7d05ee16b 100644
--- a/runsc/container/shared_volume_test.go
+++ b/runsc/container/shared_volume_test.go
@@ -173,7 +173,7 @@ func checkFile(c *Container, filename string, want []byte) error {
 	}
 	got, err := ioutil.ReadFile(cpy)
 	if err != nil {
-		return fmt.Errorf("Error reading file %q: %v", filename, err)
+		return fmt.Errorf("error reading file %q: %v", filename, err)
 	}
 	if !bytes.Equal(got, want) {
 		return fmt.Errorf("file content inside the sandbox is wrong, got: %q, want: %q", got, want)
diff --git a/runsc/container/state_file.go b/runsc/container/state_file.go
index 0399903a0..23810f593 100644
--- a/runsc/container/state_file.go
+++ b/runsc/container/state_file.go
@@ -264,10 +264,10 @@ func (s *StateFile) lockForNew() error {
 
 	// Checks if the container already exists by looking for the metadata file.
 	if _, err := os.Stat(s.statePath()); err == nil {
-		s.unlock()
+		s.unlockOrDie()
 		return fmt.Errorf("container already exists")
 	} else if !os.IsNotExist(err) {
-		s.unlock()
+		s.unlockOrDie()
 		return fmt.Errorf("looking for existing container: %v", err)
 	}
 	return nil
@@ -286,6 +286,15 @@ func (s *StateFile) unlock() error {
 	return nil
 }
 
+func (s *StateFile) unlockOrDie() {
+	if !s.flock.Locked() {
+		panic("unlock called without lock held")
+	}
+	if err := s.flock.Unlock(); err != nil {
+		panic(fmt.Sprintf("Error releasing lock on %q: %v", s.flock, err))
+	}
+}
+
 // saveLocked saves 'v' to the state file.
 //
 // Preconditions: lock() must been called before.
@@ -308,7 +317,7 @@ func (s *StateFile) load(v interface{}) error {
 	if err := s.lock(); err != nil {
 		return err
 	}
-	defer s.unlock()
+	defer s.unlockOrDie()
 
 	metaBytes, err := ioutil.ReadFile(s.statePath())
 	if err != nil {