Fixed some resource leaks leading to a "too many open files" error (#1465)

* Fixed leaking pipes in Discovery command creation

Moved the process creation at the very last moment, since StdInPipe and
StdOutPipe methods actually create an OS pipe even if the process is not
started later.

* When a discovery is quitted ensure that the process gets Wait-ed

Otherwise, the resources allocated by the process will be leaked.
(pipes, zombie process, etc.)

* fix i18n

Author: silvanocerza

arduino/discovery/discovery.go

@@ -47,6 +47,7 @@ const (
 // with the boards.
 type PluggableDiscovery struct {
 	id                   string
+	processArgs          []string
 	process              *executils.Process
 	outgoingCommandsPipe io.Writer
 	incomingMessagesChan <-chan *discoveryMessage
@@ -126,28 +127,12 @@ type Event struct {
 
 // New create and connect to the given pluggable discovery
 func New(id string, args ...string) (*PluggableDiscovery, error) {
-	proc, err := executils.NewProcess(args...)
-	if err != nil {
-		return nil, err
-	}
-	stdout, err := proc.StdoutPipe()
-	if err != nil {
-		return nil, err
-	}
-	stdin, err := proc.StdinPipe()
-	if err != nil {
-		return nil, err
-	}
-	messageChan := make(chan *discoveryMessage)
 	disc := &PluggableDiscovery{
-		id:                   id,
-		process:              proc,
-		incomingMessagesChan: messageChan,
-		outgoingCommandsPipe: stdin,
-		state:                Dead,
-		cachedPorts:          map[string]*Port{},
+		id:          id,
+		processArgs: args,
+		state:       Dead,
+		cachedPorts: map[string]*Port{},
 	}
-	go disc.jsonDecodeLoop(stdout, messageChan)
 	return disc, nil
 }
 
@@ -249,6 +234,25 @@ func (disc *PluggableDiscovery) sendCommand(command string) error {
 
 func (disc *PluggableDiscovery) runProcess() error {
 	logrus.Infof("starting discovery %s process", disc.id)
+	proc, err := executils.NewProcess(disc.processArgs...)
+	if err != nil {
+		return err
+	}
+	stdout, err := proc.StdoutPipe()
+	if err != nil {
+		return err
+	}
+	stdin, err := proc.StdinPipe()
+	if err != nil {
+		return err
+	}
+	disc.outgoingCommandsPipe = stdin
+	disc.process = proc
+
+	messageChan := make(chan *discoveryMessage)
+	disc.incomingMessagesChan = messageChan
+	go disc.jsonDecodeLoop(stdout, messageChan)
+
 	if err := disc.process.Start(); err != nil {
 		return err
 	}
@@ -264,8 +268,15 @@ func (disc *PluggableDiscovery) killProcess() error {
 	if err := disc.process.Kill(); err != nil {
 		return err
 	}
+	if err := disc.process.Wait(); err != nil {
+		return err
+	}
 	disc.statusMutex.Lock()
 	defer disc.statusMutex.Unlock()
+	if disc.eventChan != nil {
+		close(disc.eventChan)
+		disc.eventChan = nil
+	}
 	disc.state = Dead
 	logrus.Infof("killed discovery %s process", disc.id)
 	return nil
@@ -367,13 +378,7 @@ func (disc *PluggableDiscovery) Quit() error {
 	} else if msg.Message != "OK" || msg.Error {
 		return errors.Errorf(tr("command failed: %s"), msg.Message)
 	}
-	disc.statusMutex.Lock()
-	defer disc.statusMutex.Unlock()
-	if disc.eventChan != nil {
-		close(disc.eventChan)
-		disc.eventChan = nil
-	}
-	disc.state = Dead
+	disc.killProcess()
 	return nil
 }
 

arduino/discovery/discoverymanager/discoverymanager.go

@@ -120,7 +120,7 @@ func (dm *DiscoveryManager) RunAll() []error {
 func (dm *DiscoveryManager) StartAll() []error {
 	return dm.parallelize(func(d *discovery.PluggableDiscovery) error {
 		state := d.State()
-		if state != discovery.Idling || state == discovery.Running {
+		if state != discovery.Idling {
 			// Already started
 			return nil
 		}

i18n/data/en.po

@@ -21,19 +21,19 @@ msgstr "%[1]s %[2]s Version: %[3]s Commit: %[4]s Date: %[5]s"
 msgid "%[1]s folder is no longer supported! See %[2]s for more information"
 msgstr "%[1]s folder is no longer supported! See %[2]s for more information"
 
-#: arduino/discovery/discovery.go:74
+#: arduino/discovery/discovery.go:75
 msgid "%[1]s, message: %[2]s"
 msgstr "%[1]s, message: %[2]s"
 
-#: arduino/discovery/discovery.go:83
+#: arduino/discovery/discovery.go:84
 msgid "%[1]s, port: %[2]s"
 msgstr "%[1]s, port: %[2]s"
 
-#: arduino/discovery/discovery.go:80
+#: arduino/discovery/discovery.go:81
 msgid "%[1]s, ports: %[2]s"
 msgstr "%[1]s, ports: %[2]s"
 
-#: arduino/discovery/discovery.go:77
+#: arduino/discovery/discovery.go:78
 msgid "%[1]s, protocol version: %[2]d"
 msgstr "%[1]s, protocol version: %[2]d"
 
@@ -2430,12 +2430,12 @@ msgstr "board not found"
 msgid "boardname"
 msgstr "boardname"
 
-#: arduino/discovery/discovery.go:300
-#: arduino/discovery/discovery.go:321
-#: arduino/discovery/discovery.go:341
-#: arduino/discovery/discovery.go:364
-#: arduino/discovery/discovery.go:387
-#: arduino/discovery/discovery.go:410
+#: arduino/discovery/discovery.go:311
+#: arduino/discovery/discovery.go:332
+#: arduino/discovery/discovery.go:352
+#: arduino/discovery/discovery.go:375
+#: arduino/discovery/discovery.go:392
+#: arduino/discovery/discovery.go:415
 msgid "calling %[1]s: %[2]w"
 msgstr "calling %[1]s: %[2]w"
 
@@ -2477,36 +2477,36 @@ msgstr "cleaning build path"
 msgid "command"
 msgstr "command"
 
-#: arduino/discovery/discovery.go:304
-#: arduino/discovery/discovery.go:325
-#: arduino/discovery/discovery.go:345
-#: arduino/discovery/discovery.go:368
-#: arduino/discovery/discovery.go:391
-#: arduino/discovery/discovery.go:414
+#: arduino/discovery/discovery.go:315
+#: arduino/discovery/discovery.go:336
+#: arduino/discovery/discovery.go:356
+#: arduino/discovery/discovery.go:379
+#: arduino/discovery/discovery.go:396
+#: arduino/discovery/discovery.go:419
 msgid "command failed: %s"
 msgstr "command failed: %s"
 
-#: arduino/discovery/discovery.go:302
+#: arduino/discovery/discovery.go:313
 msgid "communication out of sync, expected 'hello', received '%s'"
 msgstr "communication out of sync, expected 'hello', received '%s'"
 
-#: arduino/discovery/discovery.go:389
+#: arduino/discovery/discovery.go:394
 msgid "communication out of sync, expected 'list', received '%s'"
 msgstr "communication out of sync, expected 'list', received '%s'"
 
-#: arduino/discovery/discovery.go:366
+#: arduino/discovery/discovery.go:377
 msgid "communication out of sync, expected 'quit', received '%s'"
 msgstr "communication out of sync, expected 'quit', received '%s'"
 
-#: arduino/discovery/discovery.go:323
+#: arduino/discovery/discovery.go:334
 msgid "communication out of sync, expected 'start', received '%s'"
 msgstr "communication out of sync, expected 'start', received '%s'"
 
-#: arduino/discovery/discovery.go:412
+#: arduino/discovery/discovery.go:417
 msgid "communication out of sync, expected 'start_sync', received '%s'"
 msgstr "communication out of sync, expected 'start_sync', received '%s'"
 
-#: arduino/discovery/discovery.go:343
+#: arduino/discovery/discovery.go:354
 msgid "communication out of sync, expected 'stop', received '%s'"
 msgstr "communication out of sync, expected 'stop', received '%s'"
 
@@ -2726,11 +2726,11 @@ msgstr "installing %[1]s tool: %[2]s"
 msgid "installing platform %[1]s: %[2]s"
 msgstr "installing platform %[1]s: %[2]s"
 
-#: arduino/discovery/discovery.go:190
+#: arduino/discovery/discovery.go:175
 msgid "invalid 'add' message: missing port"
 msgstr "invalid 'add' message: missing port"
 
-#: arduino/discovery/discovery.go:201
+#: arduino/discovery/discovery.go:186
 msgid "invalid 'remove' message: missing port"
 msgstr "invalid 'remove' message: missing port"
 
@@ -3065,7 +3065,7 @@ msgstr "port"
 msgid "port not found: %[1]s %[2]s"
 msgstr "port not found: %[1]s %[2]s"
 
-#: arduino/discovery/discovery.go:306
+#: arduino/discovery/discovery.go:317
 msgid "protocol version not supported: requested 1, got %d"
 msgstr "protocol version not supported: requested 1, got %d"
 
@@ -3250,7 +3250,7 @@ msgstr "the platform has no releases"
 msgid "the server responded with status %s"
 msgstr "the server responded with status %s"
 
-#: arduino/discovery/discovery.go:231
+#: arduino/discovery/discovery.go:216
 msgid "timeout waiting for message from %s"
 msgstr "timeout waiting for message from %s"