Removes locks causing deadlock

This commit removes most of the locks in the PollingMap handler as there
was combinations that caused deadlocks. Instead of doing a plain map and
doing the locking ourselves, we use sync.Map which handles it for us.
This commit is contained in:
Kristoffer Dalby 2021-08-05 22:14:37 +01:00
parent 575b15e5fa
commit 1abc68ccf4
4 changed files with 9 additions and 18 deletions

14
api.go
View file

@ -299,9 +299,7 @@ func (h *Headscale) PollNetMapHandler(c *gin.Context) {
Str("id", c.Param("id")). Str("id", c.Param("id")).
Str("machine", m.Name). Str("machine", m.Name).
Msg("Locking poll mutex") Msg("Locking poll mutex")
h.pollMu.Lock() h.clientsPolling.Store(m.ID, update)
h.clientsPolling[m.ID] = update
h.pollMu.Unlock()
log.Trace(). log.Trace().
Str("handler", "PollNetMap"). Str("handler", "PollNetMap").
Str("id", c.Param("id")). Str("id", c.Param("id")).
@ -373,9 +371,8 @@ func (h *Headscale) PollNetMapHandler(c *gin.Context) {
Str("machine", m.Name). Str("machine", m.Name).
Msg("Notifying peers") Msg("Notifying peers")
peers, _ := h.getPeers(m) peers, _ := h.getPeers(m)
h.pollMu.Lock()
for _, p := range *peers { for _, p := range *peers {
pUp, ok := h.clientsPolling[uint64(p.ID)] pUp, ok := h.clientsPolling.Load(uint64(p.ID))
if ok { if ok {
log.Info(). log.Info().
Str("handler", "PollNetMap"). Str("handler", "PollNetMap").
@ -383,7 +380,7 @@ func (h *Headscale) PollNetMapHandler(c *gin.Context) {
Str("peer", m.Name). Str("peer", m.Name).
Str("address", p.Addresses[0].String()). Str("address", p.Addresses[0].String()).
Msgf("Notifying peer %s (%s)", p.Name, p.Addresses[0]) Msgf("Notifying peer %s (%s)", p.Name, p.Addresses[0])
pUp <- []byte{} pUp.(chan []byte) <- []byte{}
} else { } else {
log.Info(). log.Info().
Str("handler", "PollNetMap"). Str("handler", "PollNetMap").
@ -392,7 +389,6 @@ func (h *Headscale) PollNetMapHandler(c *gin.Context) {
Msgf("Peer %s does not appear to be polling", p.Name) Msgf("Peer %s does not appear to be polling", p.Name)
} }
} }
h.pollMu.Unlock()
go h.keepAlive(cancelKeepAlive, pollData, mKey, req, m) go h.keepAlive(cancelKeepAlive, pollData, mKey, req, m)
@ -448,11 +444,9 @@ func (h *Headscale) PollNetMapHandler(c *gin.Context) {
now := time.Now().UTC() now := time.Now().UTC()
m.LastSeen = &now m.LastSeen = &now
h.db.Save(&m) h.db.Save(&m)
h.pollMu.Lock()
cancelKeepAlive <- []byte{} cancelKeepAlive <- []byte{}
delete(h.clientsPolling, m.ID) h.clientsPolling.Delete(m.ID)
close(update) close(update)
h.pollMu.Unlock()
return false return false
} }

3
app.go
View file

@ -59,7 +59,7 @@ type Headscale struct {
aclRules *[]tailcfg.FilterRule aclRules *[]tailcfg.FilterRule
pollMu sync.Mutex pollMu sync.Mutex
clientsPolling map[uint64]chan []byte // this is by all means a hackity hack clientsPolling sync.Map
} }
// NewHeadscale returns the Headscale app // NewHeadscale returns the Headscale app
@ -99,7 +99,6 @@ func NewHeadscale(cfg Config) (*Headscale, error) {
return nil, err return nil, err
} }
h.clientsPolling = make(map[uint64]chan []byte)
return &h, nil return &h, nil
} }

View file

@ -170,9 +170,8 @@ func (h *Headscale) checkForNamespacesPendingUpdates() {
} }
for _, m := range *machines { for _, m := range *machines {
peers, _ := h.getPeers(m) peers, _ := h.getPeers(m)
h.pollMu.Lock()
for _, p := range *peers { for _, p := range *peers {
pUp, ok := h.clientsPolling[uint64(p.ID)] pUp, ok := h.clientsPolling.Load(uint64(p.ID))
if ok { if ok {
log.Info(). log.Info().
Str("func", "checkForNamespacesPendingUpdates"). Str("func", "checkForNamespacesPendingUpdates").
@ -180,7 +179,7 @@ func (h *Headscale) checkForNamespacesPendingUpdates() {
Str("peer", m.Name). Str("peer", m.Name).
Str("address", p.Addresses[0].String()). Str("address", p.Addresses[0].String()).
Msgf("Notifying peer %s (%s)", p.Name, p.Addresses[0]) Msgf("Notifying peer %s (%s)", p.Name, p.Addresses[0])
pUp <- []byte{} pUp.(chan []byte) <- []byte{}
} else { } else {
log.Info(). log.Info().
Str("func", "checkForNamespacesPendingUpdates"). Str("func", "checkForNamespacesPendingUpdates").
@ -189,7 +188,6 @@ func (h *Headscale) checkForNamespacesPendingUpdates() {
Msgf("Peer %s does not appear to be polling", p.Name) Msgf("Peer %s does not appear to be polling", p.Name)
} }
} }
h.pollMu.Unlock()
} }
} }
newV, err := h.getValue("namespaces_pending_updates") newV, err := h.getValue("namespaces_pending_updates")

View file

@ -52,8 +52,8 @@ func (h *Headscale) EnableNodeRoute(namespace string, nodeName string, routeStr
peers, _ := h.getPeers(*m) peers, _ := h.getPeers(*m)
h.pollMu.Lock() h.pollMu.Lock()
for _, p := range *peers { for _, p := range *peers {
if pUp, ok := h.clientsPolling[uint64(p.ID)]; ok { if pUp, ok := h.clientsPolling.Load(uint64(p.ID)); ok {
pUp <- []byte{} pUp.(chan []byte) <- []byte{}
} }
} }
h.pollMu.Unlock() h.pollMu.Unlock()