Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(458)

Delta Between Two Patch Sets: src/pkg/os/exec/exec.go

Issue 6789043: code review 6789043: os/exec: fix fd leak on error
Left Patch Set: Created 11 years, 5 months ago
Right Patch Set: diff -r 57f70857cf91 https://go.googlecode.com/hg/ Created 11 years, 5 months ago
Left:
Right:
Use n/p to move between diff chunks; N/P to move between comments. Please Sign in to add in-line comments.
Jump to:
Right: Side by side diff | Download
« no previous file with change/comment | « no previous file | src/pkg/os/exec/exec_test.go » ('j') | src/pkg/os/exec/exec_test.go » ('J')
Toggle Intra-line Diffs ('i') | Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
LEFTRIGHT
(no file at all)
1 // Copyright 2009 The Go Authors. All rights reserved. 1 // Copyright 2009 The Go Authors. All rights reserved.
2 // Use of this source code is governed by a BSD-style 2 // Use of this source code is governed by a BSD-style
3 // license that can be found in the LICENSE file. 3 // license that can be found in the LICENSE file.
4 4
5 // Package exec runs external commands. It wraps os.StartProcess to make it 5 // Package exec runs external commands. It wraps os.StartProcess to make it
6 // easier to remap stdin and stdout, connect I/O with pipes, and do other 6 // easier to remap stdin and stdout, connect I/O with pipes, and do other
7 // adjustments. 7 // adjustments.
8 package exec 8 package exec
9 9
10 import ( 10 import (
(...skipping 69 matching lines...) Expand 10 before | Expand all | Expand 10 after
80 Process *os.Process 80 Process *os.Process
81 81
82 // ProcessState contains information about an exited process, 82 // ProcessState contains information about an exited process,
83 // available after a call to Wait or Run. 83 // available after a call to Wait or Run.
84 ProcessState *os.ProcessState 84 ProcessState *os.ProcessState
85 85
86 err error // last error (from LookPath, stdin, stdout, stder r) 86 err error // last error (from LookPath, stdin, stdout, stder r)
87 finished bool // when Wait was called 87 finished bool // when Wait was called
88 childFiles []*os.File 88 childFiles []*os.File
89 closeAfterStart []io.Closer 89 closeAfterStart []io.Closer
90 closeAfterWait []io.Closer
91 goroutine []func() error 90 goroutine []func() error
92 errch chan error // one send per goroutine 91 errch chan error // one send per goroutine
93 } 92 }
94 93
95 // Command returns the Cmd struct to execute the named program with 94 // Command returns the Cmd struct to execute the named program with
96 // the given arguments. 95 // the given arguments.
97 // 96 //
98 // It sets Path and Args in the returned structure and zeroes the 97 // It sets Path and Args in the returned structure and zeroes the
99 // other fields. 98 // other fields.
100 // 99 //
(...skipping 52 matching lines...) Expand 10 before | Expand all | Expand 10 after
153 if f, ok := c.Stdin.(*os.File); ok { 152 if f, ok := c.Stdin.(*os.File); ok {
154 return f, nil 153 return f, nil
155 } 154 }
156 155
157 pr, pw, err := os.Pipe() 156 pr, pw, err := os.Pipe()
158 if err != nil { 157 if err != nil {
159 return 158 return
160 } 159 }
161 160
162 c.closeAfterStart = append(c.closeAfterStart, pr) 161 c.closeAfterStart = append(c.closeAfterStart, pr)
163 c.closeAfterWait = append(c.closeAfterWait, pw)
164 c.goroutine = append(c.goroutine, func() error { 162 c.goroutine = append(c.goroutine, func() error {
165 _, err := io.Copy(pw, c.Stdin) 163 _, err := io.Copy(pw, c.Stdin)
166 if err1 := pw.Close(); err == nil { 164 if err1 := pw.Close(); err == nil {
167 err = err1 165 err = err1
168 } 166 }
169 return err 167 return err
170 }) 168 })
171 return pr, nil 169 return pr, nil
172 } 170 }
173 171
(...skipping 21 matching lines...) Expand all
195 if f, ok := w.(*os.File); ok { 193 if f, ok := w.(*os.File); ok {
196 return f, nil 194 return f, nil
197 } 195 }
198 196
199 pr, pw, err := os.Pipe() 197 pr, pw, err := os.Pipe()
200 if err != nil { 198 if err != nil {
201 return 199 return
202 } 200 }
203 201
204 c.closeAfterStart = append(c.closeAfterStart, pw) 202 c.closeAfterStart = append(c.closeAfterStart, pw)
205 c.closeAfterWait = append(c.closeAfterWait, pr)
206 c.goroutine = append(c.goroutine, func() error { 203 c.goroutine = append(c.goroutine, func() error {
207 _, err := io.Copy(w, pr) 204 _, err := io.Copy(w, pr)
205 pr.Close()
208 return err 206 return err
209 }) 207 })
210 return pw, nil 208 return pw, nil
211 } 209 }
212 210
213 func (c *Cmd) closeDescriptors(closers []io.Closer) { 211 func (c *Cmd) closeDescriptors(closers []io.Closer) {
214 for _, fd := range closers { 212 for _, fd := range closers {
215 fd.Close() 213 fd.Close()
216 } 214 }
217 } 215 }
(...skipping 10 matching lines...) Expand all
228 func (c *Cmd) Run() error { 226 func (c *Cmd) Run() error {
229 if err := c.Start(); err != nil { 227 if err := c.Start(); err != nil {
230 return err 228 return err
231 } 229 }
232 return c.Wait() 230 return c.Wait()
233 } 231 }
234 232
235 // Start starts the specified command but does not wait for it to complete. 233 // Start starts the specified command but does not wait for it to complete.
236 func (c *Cmd) Start() error { 234 func (c *Cmd) Start() error {
237 if c.err != nil { 235 if c.err != nil {
236 c.closeDescriptors(c.closeAfterStart)
238 return c.err 237 return c.err
239 } 238 }
240 if c.Process != nil { 239 if c.Process != nil {
241 return errors.New("exec: already started") 240 return errors.New("exec: already started")
242 } 241 }
243 242
244 type F func(*Cmd) (*os.File, error) 243 type F func(*Cmd) (*os.File, error)
245 for _, setupFd := range []F{(*Cmd).stdin, (*Cmd).stdout, (*Cmd).stderr} { 244 for _, setupFd := range []F{(*Cmd).stdin, (*Cmd).stdout, (*Cmd).stderr} {
246 fd, err := setupFd(c) 245 fd, err := setupFd(c)
247 if err != nil { 246 if err != nil {
248 c.closeDescriptors(c.closeAfterStart) 247 c.closeDescriptors(c.closeAfterStart)
249 c.closeDescriptors(c.closeAfterWait)
250 return err 248 return err
251 } 249 }
252 c.childFiles = append(c.childFiles, fd) 250 c.childFiles = append(c.childFiles, fd)
253 } 251 }
254 c.childFiles = append(c.childFiles, c.ExtraFiles...) 252 c.childFiles = append(c.childFiles, c.ExtraFiles...)
255 253
256 var err error 254 var err error
257 c.Process, err = os.StartProcess(c.Path, c.argv(), &os.ProcAttr{ 255 c.Process, err = os.StartProcess(c.Path, c.argv(), &os.ProcAttr{
258 Dir: c.Dir, 256 Dir: c.Dir,
259 Files: c.childFiles, 257 Files: c.childFiles,
260 Env: c.envv(), 258 Env: c.envv(),
261 Sys: c.SysProcAttr, 259 Sys: c.SysProcAttr,
262 }) 260 })
263 if err != nil { 261 if err != nil {
264 c.closeDescriptors(c.closeAfterStart) 262 c.closeDescriptors(c.closeAfterStart)
265 c.closeDescriptors(c.closeAfterWait)
266 return err 263 return err
267 } 264 }
268 265
269 c.closeDescriptors(c.closeAfterStart) 266 c.closeDescriptors(c.closeAfterStart)
270 267
271 c.errch = make(chan error, len(c.goroutine)) 268 c.errch = make(chan error, len(c.goroutine))
272 for _, fn := range c.goroutine { 269 for _, fn := range c.goroutine {
273 go func(fn func() error) { 270 go func(fn func() error) {
274 c.errch <- fn() 271 c.errch <- fn()
275 }(fn) 272 }(fn)
(...skipping 32 matching lines...) Expand 10 before | Expand all | Expand 10 after
308 state, err := c.Process.Wait() 305 state, err := c.Process.Wait()
309 c.ProcessState = state 306 c.ProcessState = state
310 307
311 var copyError error 308 var copyError error
312 for _ = range c.goroutine { 309 for _ = range c.goroutine {
313 if err := <-c.errch; err != nil && copyError == nil { 310 if err := <-c.errch; err != nil && copyError == nil {
314 copyError = err 311 copyError = err
315 } 312 }
316 } 313 }
317 314
318 c.closeDescriptors(c.closeAfterWait)
319
320 if err != nil { 315 if err != nil {
321 return err 316 return err
322 } else if !state.Success() { 317 } else if !state.Success() {
323 return &ExitError{state} 318 return &ExitError{state}
324 } 319 }
325 320
326 return copyError 321 return copyError
327 } 322 }
328 323
329 // Output runs the command and returns its standard output. 324 // Output runs the command and returns its standard output.
(...skipping 31 matching lines...) Expand 10 before | Expand all | Expand 10 after
361 } 356 }
362 if c.Process != nil { 357 if c.Process != nil {
363 return nil, errors.New("exec: StdinPipe after process started") 358 return nil, errors.New("exec: StdinPipe after process started")
364 } 359 }
365 pr, pw, err := os.Pipe() 360 pr, pw, err := os.Pipe()
366 if err != nil { 361 if err != nil {
367 return nil, err 362 return nil, err
368 } 363 }
369 c.Stdin = pr 364 c.Stdin = pr
370 c.closeAfterStart = append(c.closeAfterStart, pr) 365 c.closeAfterStart = append(c.closeAfterStart, pr)
371 c.closeAfterWait = append(c.closeAfterWait, pw)
372 return pw, nil 366 return pw, nil
373 } 367 }
374 368
375 // StdoutPipe returns a pipe that will be connected to the command's 369 // StdoutPipe returns a pipe that will be connected to the command's
376 // standard output when the command starts. 370 // standard output when the command starts.
377 // The pipe will be closed automatically after Wait sees the command exit.
378 func (c *Cmd) StdoutPipe() (io.ReadCloser, error) { 371 func (c *Cmd) StdoutPipe() (io.ReadCloser, error) {
379 if c.Stdout != nil { 372 if c.Stdout != nil {
380 return nil, errors.New("exec: Stdout already set") 373 return nil, errors.New("exec: Stdout already set")
381 } 374 }
382 if c.Process != nil { 375 if c.Process != nil {
383 return nil, errors.New("exec: StdoutPipe after process started") 376 return nil, errors.New("exec: StdoutPipe after process started")
384 } 377 }
385 pr, pw, err := os.Pipe() 378 pr, pw, err := os.Pipe()
386 if err != nil { 379 if err != nil {
387 return nil, err 380 return nil, err
388 } 381 }
389 c.Stdout = pw 382 c.Stdout = pw
390 c.closeAfterStart = append(c.closeAfterStart, pw) 383 c.closeAfterStart = append(c.closeAfterStart, pw)
391 c.closeAfterWait = append(c.closeAfterWait, pr)
392 return pr, nil 384 return pr, nil
393 } 385 }
394 386
395 // StderrPipe returns a pipe that will be connected to the command's 387 // StderrPipe returns a pipe that will be connected to the command's
396 // standard error when the command starts. 388 // standard error when the command starts.
397 // The pipe will be closed automatically after Wait sees the command exit.
398 func (c *Cmd) StderrPipe() (io.ReadCloser, error) { 389 func (c *Cmd) StderrPipe() (io.ReadCloser, error) {
399 if c.Stderr != nil { 390 if c.Stderr != nil {
400 return nil, errors.New("exec: Stderr already set") 391 return nil, errors.New("exec: Stderr already set")
401 } 392 }
402 if c.Process != nil { 393 if c.Process != nil {
403 return nil, errors.New("exec: StderrPipe after process started") 394 return nil, errors.New("exec: StderrPipe after process started")
404 } 395 }
405 pr, pw, err := os.Pipe() 396 pr, pw, err := os.Pipe()
406 if err != nil { 397 if err != nil {
407 return nil, err 398 return nil, err
408 } 399 }
409 c.Stderr = pw 400 c.Stderr = pw
410 c.closeAfterStart = append(c.closeAfterStart, pw) 401 c.closeAfterStart = append(c.closeAfterStart, pw)
411 c.closeAfterWait = append(c.closeAfterWait, pr)
412 return pr, nil 402 return pr, nil
413 } 403 }
LEFTRIGHT

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b