Skip to content

Commit

Permalink
Fixed send-on-closed channel bug in DiscoveryManager (#1472)
Browse files Browse the repository at this point in the history
  • Loading branch information
cmaglie committed Sep 24, 2021
1 parent 959f8e8 commit be520ef
Show file tree
Hide file tree
Showing 3 changed files with 28 additions and 36 deletions.
36 changes: 14 additions & 22 deletions arduino/discovery/discoverymanager/discoverymanager.go
Expand Up @@ -27,28 +27,22 @@ import (
// DiscoveryManager is required to handle multiple pluggable-discovery that
// may be shared across platforms
type DiscoveryManager struct {
discoveries map[string]*discovery.PluggableDiscovery
globalEventCh chan *discovery.Event
discoveries map[string]*discovery.PluggableDiscovery
}

var tr = i18n.Tr

// New creates a new DiscoveryManager
func New() *DiscoveryManager {
return &DiscoveryManager{
discoveries: map[string]*discovery.PluggableDiscovery{},
globalEventCh: nil,
discoveries: map[string]*discovery.PluggableDiscovery{},
}
}

// Clear resets the DiscoveryManager to its initial state
func (dm *DiscoveryManager) Clear() {
dm.QuitAll()
dm.discoveries = map[string]*discovery.PluggableDiscovery{}
if dm.globalEventCh != nil {
close(dm.globalEventCh)
dm.globalEventCh = nil
}
}

// IDs returns the list of discoveries' ids in this DiscoveryManager
Expand Down Expand Up @@ -134,9 +128,8 @@ func (dm *DiscoveryManager) StartAll() []error {
// StartSyncAll the discoveries for this DiscoveryManager,
// returns an error for each discovery failing to start syncing
func (dm *DiscoveryManager) StartSyncAll() (<-chan *discovery.Event, []error) {
if dm.globalEventCh == nil {
dm.globalEventCh = make(chan *discovery.Event, 5)
}
eventSink := make(chan *discovery.Event, 5)
var wg sync.WaitGroup
errs := dm.parallelize(func(d *discovery.PluggableDiscovery) error {
state := d.State()
if state != discovery.Idling || state == discovery.Syncing {
Expand All @@ -148,14 +141,22 @@ func (dm *DiscoveryManager) StartSyncAll() (<-chan *discovery.Event, []error) {
if err != nil {
return fmt.Errorf(tr("start syncing discovery %[1]s: %[2]w"), d.GetID(), err)
}

wg.Add(1)
go func() {
for ev := range eventCh {
dm.globalEventCh <- ev
eventSink <- ev
}
wg.Done()
}()
return nil
})
return dm.globalEventCh, errs
go func() {
wg.Wait()
eventSink <- &discovery.Event{Type: "quit"}
close(eventSink)
}()
return eventSink, errs
}

// StopAll the discoveries for this DiscoveryManager,
Expand Down Expand Up @@ -189,15 +190,6 @@ func (dm *DiscoveryManager) QuitAll() []error {
}
return nil
})
// Close the global channel only if there were no errors
// quitting all alive discoveries
if len(errs) == 0 && dm.globalEventCh != nil {
// Let events consumers that discoveries are quitting no more events
// will be sent on this channel
dm.globalEventCh <- &discovery.Event{Type: "quit"}
close(dm.globalEventCh)
dm.globalEventCh = nil
}
return errs
}

Expand Down
14 changes: 7 additions & 7 deletions i18n/data/en.po
Expand Up @@ -2541,7 +2541,7 @@ msgstr "destination dir %s already exists, cannot install"
msgid "directory doesn't exist: %s"
msgstr "directory doesn't exist: %s"

#: arduino/discovery/discoverymanager/discoverymanager.go:112
#: arduino/discovery/discoverymanager/discoverymanager.go:106
msgid "discovery %[1]s process not started: %[2]w"
msgstr "discovery %[1]s process not started: %[2]w"

Expand Down Expand Up @@ -2840,7 +2840,7 @@ msgstr "library not valid"
msgid "library path does not exist: %s"
msgstr "library path does not exist: %s"

#: arduino/discovery/discoverymanager/discoverymanager.go:225
#: arduino/discovery/discoverymanager/discoverymanager.go:217
msgid "listing ports from discovery %[1]s: %[2]w"
msgstr "listing ports from discovery %[1]s: %[2]w"

Expand Down Expand Up @@ -3038,7 +3038,7 @@ msgstr "platform not installed"
msgid "please use --build-property instead."
msgstr "please use --build-property instead."

#: arduino/discovery/discoverymanager/discoverymanager.go:67
#: arduino/discovery/discoverymanager/discoverymanager.go:61
msgid "pluggable discovery already added: %s"
msgstr "pluggable discovery already added: %s"

Expand All @@ -3055,7 +3055,7 @@ msgstr "port not found: %[1]s %[2]s"
msgid "protocol version not supported: requested 1, got %d"
msgstr "protocol version not supported: requested 1, got %d"

#: arduino/discovery/discoverymanager/discoverymanager.go:188
#: arduino/discovery/discoverymanager/discoverymanager.go:189
msgid "quitting discovery %[1]s: %[2]w"
msgstr "quitting discovery %[1]s: %[2]w"

Expand Down Expand Up @@ -3193,19 +3193,19 @@ msgstr "skipping loading of boards %s: malformed custom board options"
msgid "source is not a directory"
msgstr "source is not a directory"

#: arduino/discovery/discoverymanager/discoverymanager.go:149
#: arduino/discovery/discoverymanager/discoverymanager.go:142
msgid "start syncing discovery %[1]s: %[2]w"
msgstr "start syncing discovery %[1]s: %[2]w"

#: arduino/discovery/discoverymanager/discoverymanager.go:128
#: arduino/discovery/discoverymanager/discoverymanager.go:122
msgid "starting discovery %[1]s: %[2]w"
msgstr "starting discovery %[1]s: %[2]w"

#: commands/board/list.go:302
msgid "stopping discoveries: %s"
msgstr "stopping discoveries: %s"

#: arduino/discovery/discoverymanager/discoverymanager.go:172
#: arduino/discovery/discoverymanager/discoverymanager.go:173
msgid "stopping discovery %[1]s: %[2]w"
msgstr "stopping discovery %[1]s: %[2]w"

Expand Down
14 changes: 7 additions & 7 deletions i18n/rice-box.go

Large diffs are not rendered by default.

0 comments on commit be520ef

Please sign in to comment.