Skip to content

Commit

Permalink
Fixed possible deadlock when updating route permissions
Browse files Browse the repository at this point in the history
This bug is only in master, not in any public release.

Signed-off-by: Ivan Kozlovic <ivan@synadia.com>
  • Loading branch information
kozlovic committed Dec 7, 2018
1 parent 001be75 commit c57ff0e
Showing 1 changed file with 18 additions and 10 deletions.
28 changes: 18 additions & 10 deletions server/route.go
Expand Up @@ -351,6 +351,16 @@ func (c *client) sendConnect(tlsRequired bool) {

// Process the info message if we are a route.
func (c *client) processRouteInfo(info *Info) {
// We may need to update route permissions and will need the account
// sublist. Since getting the account requires server lock, do the
// lookup now.

// FIXME(dlc) - Add account scoping.
gacc := c.srv.globalAccount()
gacc.mu.RLock()
sl := gacc.sl
gacc.mu.RUnlock()

c.mu.Lock()
// Connection can be closed at any time (by auth timeout, etc).
// Does not make sense to continue here if connection is gone.
Expand Down Expand Up @@ -409,7 +419,7 @@ func (c *client) processRouteInfo(info *Info) {
// If this is an update due to config reload on the remote server,
// need to possibly send local subs to the remote server.
if c.flags.isSet(infoReceived) {
s.updateRemoteRoutePerms(c, info)
c.updateRemoteRoutePerms(sl, info)
c.mu.Unlock()
return
}
Expand Down Expand Up @@ -486,34 +496,32 @@ func (c *client) processRouteInfo(info *Info) {
// Possibly sends local subscriptions interest to this route
// based on changes in the remote's Export permissions.
// Lock assumed held on entry
func (s *Server) updateRemoteRoutePerms(route *client, info *Info) {
func (c *client) updateRemoteRoutePerms(sl *Sublist, info *Info) {
// Interested only on Export permissions for the remote server.
// Create "fake" clients that we will use to check permissions
// using the old permissions...
oldPerms := &RoutePermissions{Export: route.opts.Export}
oldPerms := &RoutePermissions{Export: c.opts.Export}
oldPermsTester := &client{}
oldPermsTester.setRoutePermissions(oldPerms)
// and the new ones.
newPerms := &RoutePermissions{Export: info.Export}
newPermsTester := &client{}
newPermsTester.setRoutePermissions(newPerms)

route.opts.Import = info.Import
route.opts.Export = info.Export
c.opts.Import = info.Import
c.opts.Export = info.Export

var (
_localSubs [4096]*subscription
localSubs = _localSubs[:0]
)
// FIXME(dlc) - Add account scoping.
gacc := s.globalAccount()
gacc.sl.localSubs(&localSubs)
sl.localSubs(&localSubs)

route.sendRouteSubProtos(localSubs, false, func(sub *subscription) bool {
c.sendRouteSubProtos(localSubs, false, func(sub *subscription) bool {
subj := string(sub.subject)
// If the remote can now export but could not before, and this server can import this
// subject, then send SUB protocol.
if newPermsTester.canExport(subj) && !oldPermsTester.canExport(subj) && route.canImport(subj) {
if newPermsTester.canExport(subj) && !oldPermsTester.canExport(subj) && c.canImport(subj) {
return true
}
return false
Expand Down

0 comments on commit c57ff0e

Please sign in to comment.