Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Sep 23, 2021
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
c4e19c1
Fixed leaking pipes in Discovery command creation
cmaglie Sep 23, 2021
a411b70
When a discovery is quitted ensure that the process gets Wait-ed
cmaglie Sep 23, 2021
3e75e71
fix i18n
cmaglie Sep 23, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
59 changes: 32 additions & 27 deletions arduino/discovery/discovery.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ const (
// with the boards.
type PluggableDiscovery struct {
id string
processArgs []string
process *executils.Process
outgoingCommandsPipe io.Writer
incomingMessagesChan <-chan *discoveryMessage
Expand Down Expand Up @@ -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
}

Expand Down Expand Up @@ -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
}
Expand All @@ -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
Expand Down Expand Up @@ -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
}

Expand Down
2 changes: 1 addition & 1 deletion arduino/discovery/discoverymanager/discoverymanager.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
52 changes: 26 additions & 26 deletions i18n/data/en.po
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,19 +13,19 @@ msgstr "%[1]s folder is no longer supported! See %[2]s for more information"
msgid "%[1]s is required but %[2]s is currently installed."
msgstr "%[1]s is required but %[2]s is currently installed."

#: 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"

Expand Down Expand Up @@ -2388,12 +2388,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
#: arduino/monitor/monitor.go:246
msgid "calling %[1]s: %[2]w"
msgstr "calling %[1]s: %[2]w"
Expand Down Expand Up @@ -2441,12 +2441,12 @@ 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
#: arduino/monitor/monitor.go:250
#: arduino/monitor/monitor.go:270
#: arduino/monitor/monitor.go:287
Expand All @@ -2467,32 +2467,32 @@ msgstr "communication out of sync, expected 'configure', received '%s'"
msgid "communication out of sync, expected 'describe', received '%s'"
msgstr "communication out of sync, expected 'describe', received '%s'"

#: arduino/discovery/discovery.go:302
#: arduino/discovery/discovery.go:313
#: arduino/monitor/monitor.go:248
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/monitor/monitor.go:321
msgid "communication out of sync, expected 'open', received '%s'"
msgstr "communication out of sync, expected 'open', 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'"

Expand Down Expand Up @@ -2706,11 +2706,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"

Expand Down Expand Up @@ -3050,7 +3050,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
#: arduino/monitor/monitor.go:252
msgid "protocol version not supported: requested 1, got %d"
msgstr "protocol version not supported: requested 1, got %d"
Expand Down Expand Up @@ -3233,7 +3233,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"

Expand Down
14 changes: 7 additions & 7 deletions i18n/rice-box.go
View file
Open in desktop

Large diffs are not rendered by default.