Merge pull request opencontainers#2038 from imxyb/defer-destroy

Mrunal Patel · web-flow · commit eb4aeed24ffb · 2019-05-07T15:48:14.000-07:00
`r.destroy` can defer exec in `runner.run` method.
diff --git a/utils_linux.go b/utils_linux.go
@@ -269,13 +269,17 @@ type runner struct {
 }
 
 func (r *runner) run(config *specs.Process) (int, error) {
-	if err := r.checkTerminal(config); err != nil {
-		r.destroy()
+	var err error
+	defer func() {
+		if err != nil {
+			r.destroy()
+		}
+	}()
+	if err = r.checkTerminal(config); err != nil {
 		return -1, err
 	}
 	process, err := newProcess(*config, r.init, r.logLevel)
 	if err != nil {
-		r.destroy()
 		return -1, err
 	}
 	if len(r.listenFDs) > 0 {
@@ -284,21 +288,18 @@ func (r *runner) run(config *specs.Process) (int, error) {
 	}
 	baseFd := 3 + len(process.ExtraFiles)
 	for i := baseFd; i < baseFd+r.preserveFDs; i++ {
-		_, err := os.Stat(fmt.Sprintf("/proc/self/fd/%d", i))
+		_, err = os.Stat(fmt.Sprintf("/proc/self/fd/%d", i))
 		if err != nil {
-			r.destroy()
 			return -1, errors.Wrapf(err, "please check that preserved-fd %d (of %d) is present", i-baseFd, r.preserveFDs)
 		}
 		process.ExtraFiles = append(process.ExtraFiles, os.NewFile(uintptr(i), "PreserveFD:"+strconv.Itoa(i)))
 	}
 	rootuid, err := r.container.Config().HostRootUID()
 	if err != nil {
-		r.destroy()
 		return -1, err
 	}
 	rootgid, err := r.container.Config().HostRootGID()
 	if err != nil {
-		r.destroy()
 		return -1, err
 	}
 	var (
@@ -310,7 +311,6 @@ func (r *runner) run(config *specs.Process) (int, error) {
 	handler := newSignalHandler(r.enableSubreaper, r.notifySocket)
 	tty, err := setupIO(process, rootuid, rootgid, config.Terminal, detach, r.consoleSocket)
 	if err != nil {
-		r.destroy()
 		return -1, err
 	}
 	defer tty.Close()
@@ -326,23 +326,19 @@ func (r *runner) run(config *specs.Process) (int, error) {
 		panic("Unknown action")
 	}
 	if err != nil {
-		r.destroy()
 		return -1, err
 	}
-	if err := tty.waitConsole(); err != nil {
+	if err = tty.waitConsole(); err != nil {
 		r.terminate(process)
-		r.destroy()
 		return -1, err
 	}
 	if err = tty.ClosePostStart(); err != nil {
 		r.terminate(process)
-		r.destroy()
 		return -1, err
 	}
 	if r.pidFile != "" {
 		if err = createPidFile(r.pidFile, process); err != nil {
 			r.terminate(process)
-			r.destroy()
 			return -1, err
 		}
 	}

Original file line number	Diff line number	Diff line change
`@@ -269,13 +269,17 @@ type runner struct {`
`269`	`269`	`}`
`270`	`270`
`271`	`271`	`func (r runner) run(config specs.Process) (int, error) {`
`272`		`- if err := r.checkTerminal(config); err != nil {`
`273`		`- r.destroy()`
	`272`	`+ var err error`
	`273`	`+ defer func() {`
	`274`	`+ if err != nil {`
	`275`	`+ r.destroy()`
	`276`	`+ }`
	`277`	`+ }()`
	`278`	`+ if err = r.checkTerminal(config); err != nil {`
`274`	`279`	`return -1, err`
`275`	`280`	`}`
`276`	`281`	`process, err := newProcess(*config, r.init, r.logLevel)`
`277`	`282`	`if err != nil {`
`278`		`- r.destroy()`
`279`	`283`	`return -1, err`
`280`	`284`	`}`
`281`	`285`	`if len(r.listenFDs) > 0 {`
`@@ -284,21 +288,18 @@ func (r runner) run(config specs.Process) (int, error) {`
`284`	`288`	`}`
`285`	`289`	`baseFd := 3 + len(process.ExtraFiles)`
`286`	`290`	`for i := baseFd; i < baseFd+r.preserveFDs; i++ {`
`287`		`- _, err := os.Stat(fmt.Sprintf("/proc/self/fd/%d", i))`
	`291`	`+ _, err = os.Stat(fmt.Sprintf("/proc/self/fd/%d", i))`
`288`	`292`	`if err != nil {`
`289`		`- r.destroy()`
`290`	`293`	`return -1, errors.Wrapf(err, "please check that preserved-fd %d (of %d) is present", i-baseFd, r.preserveFDs)`
`291`	`294`	`}`
`292`	`295`	`process.ExtraFiles = append(process.ExtraFiles, os.NewFile(uintptr(i), "PreserveFD:"+strconv.Itoa(i)))`
`293`	`296`	`}`
`294`	`297`	`rootuid, err := r.container.Config().HostRootUID()`
`295`	`298`	`if err != nil {`
`296`		`- r.destroy()`
`297`	`299`	`return -1, err`
`298`	`300`	`}`
`299`	`301`	`rootgid, err := r.container.Config().HostRootGID()`
`300`	`302`	`if err != nil {`
`301`		`- r.destroy()`
`302`	`303`	`return -1, err`
`303`	`304`	`}`
`304`	`305`	`var (`
`@@ -310,7 +311,6 @@ func (r runner) run(config specs.Process) (int, error) {`
`310`	`311`	`handler := newSignalHandler(r.enableSubreaper, r.notifySocket)`
`311`	`312`	`tty, err := setupIO(process, rootuid, rootgid, config.Terminal, detach, r.consoleSocket)`
`312`	`313`	`if err != nil {`
`313`		`- r.destroy()`
`314`	`314`	`return -1, err`
`315`	`315`	`}`
`316`	`316`	`defer tty.Close()`
`@@ -326,23 +326,19 @@ func (r runner) run(config specs.Process) (int, error) {`
`326`	`326`	`panic("Unknown action")`
`327`	`327`	`}`
`328`	`328`	`if err != nil {`
`329`		`- r.destroy()`
`330`	`329`	`return -1, err`
`331`	`330`	`}`
`332`		`- if err := tty.waitConsole(); err != nil {`
	`331`	`+ if err = tty.waitConsole(); err != nil {`
`333`	`332`	`r.terminate(process)`
`334`		`- r.destroy()`
`335`	`333`	`return -1, err`
`336`	`334`	`}`
`337`	`335`	`if err = tty.ClosePostStart(); err != nil {`
`338`	`336`	`r.terminate(process)`
`339`		`- r.destroy()`
`340`	`337`	`return -1, err`
`341`	`338`	`}`
`342`	`339`	`if r.pidFile != "" {`
`343`	`340`	`if err = createPidFile(r.pidFile, process); err != nil {`
`344`	`341`	`r.terminate(process)`
`345`		`- r.destroy()`
`346`	`342`	`return -1, err`
`347`	`343`	`}`
`348`	`344`	`}`