Skip to content

Commit

Permalink
Fix a number of potential and actual race conditions
Browse files Browse the repository at this point in the history
Fixes #4
  • Loading branch information
pdf committed Mar 25, 2016
1 parent 2d60aae commit cc4aa3c
Show file tree
Hide file tree
Showing 5 changed files with 168 additions and 83 deletions.
9 changes: 6 additions & 3 deletions client.go
Original file line number Diff line number Diff line change
Expand Up @@ -165,10 +165,11 @@ func (c *Client) removeDeviceByID(id uint64) error {
// GetLocations returns a slice of all locations known to the client, or
// common.ErrNotFound if no locations are currently known.
func (c *Client) GetLocations() (locations []common.Location, err error) {
c.RLock()
if len(c.locations) == 0 {
c.RUnlock()
return locations, common.ErrNotFound
}
c.RLock()
for _, location := range c.locations {
locations = append(locations, location)
}
Expand Down Expand Up @@ -258,10 +259,11 @@ func (c *Client) GetLocationByLabel(label string) (common.Location, error) {
// GetGroups returns a slice of all groups known to the client, or
// common.ErrNotFound if no groups are currently known.
func (c *Client) GetGroups() (groups []common.Group, err error) {
c.RLock()
if len(c.groups) == 0 {
c.RUnlock()
return groups, common.ErrNotFound
}
c.RLock()
for _, group := range c.groups {
groups = append(groups, group)
}
Expand Down Expand Up @@ -351,10 +353,11 @@ func (c *Client) GetGroupByLabel(label string) (common.Group, error) {
// GetDevices returns a slice of all devices known to the client, or
// common.ErrNotFound if no devices are currently known.
func (c *Client) GetDevices() (devices []common.Device, err error) {
c.RLock()
if len(c.devices) == 0 {
c.RUnlock()
return devices, common.ErrNotFound
}
c.RLock()
for _, device := range c.devices {
devices = append(devices, device)
}
Expand Down
10 changes: 5 additions & 5 deletions protocol/v2.go
Original file line number Diff line number Diff line change
Expand Up @@ -93,17 +93,17 @@ func (p *V2) init() error {
if err != nil {
return err
}
p.broadcast = &device.Light{Device: *broadcastDev}
p.broadcast = &device.Light{Device: broadcastDev}
broadcastSub, err := p.broadcast.NewSubscription()
if err != nil {
return err
}
go p.broadcastLimiter(broadcastSub.Events())
p.devices = make(map[uint64]device.GenericDevice)
p.locations = make(map[string]*device.Location)
p.groups = make(map[string]*device.Group)
p.subscriptions = make(map[string]*common.Subscription)
p.quitChan = make(chan struct{})
go p.broadcastLimiter(broadcastSub.Events())
go p.dispatcher()
p.initialized = true

Expand Down Expand Up @@ -570,13 +570,13 @@ func (p *V2) lightOrDev(dev device.GenericDevice) device.GenericDevice {
switch product {
case device.ProductLifxOriginal1000, device.ProductLifxColor650, device.ProductLifxWhite800LowVoltage, device.ProductLifxWhite800HighVoltage, device.ProductLifxWhite900BR30, device.ProductLifxColor1000BR30, device.ProductLifxColor1000:
p.Lock()
// Need to figure if there's a way to do this without being racey on the
// lock inside the dev
d := dev.(*device.Device)
l := &device.Light{Device: *d}
d.Lock()
l := &device.Light{Device: d}
common.Log.Debugf("Device is a light: %v\n", l.ID())
// Replace the known dev with our constructed light
p.devices[l.ID()] = l
d.Unlock()
p.Unlock()
if err := l.Get(); err != nil {
common.Log.Debugf("Failed getting light state: %v\n", err)
Expand Down
Loading

0 comments on commit cc4aa3c

Please sign in to comment.