Skip to content

Commit

Permalink
[MM-50990] Fix potential rtcd client leak (#350)
Browse files Browse the repository at this point in the history
* Fix client connection leak on version checking failure

* Skip check for master builds

* Abort reconnection if host is either missing or flagged
  • Loading branch information
streamer45 committed Mar 1, 2023
1 parent 0a9d75c commit 1c423ed
Showing 1 changed file with 36 additions and 19 deletions.
55 changes: 36 additions & 19 deletions server/rtcd.go
Original file line number Diff line number Diff line change
Expand Up @@ -88,10 +88,6 @@ func (p *Plugin) newRTCDClientManager(rtcdURL string) (m *rtcdClientManager, err
return nil, err
}

if err := m.versionCheck(client); err != nil {
return nil, fmt.Errorf("version compatibility check failed: %w", err)
}

if err := m.addHost(ip.String(), client); err != nil {
return nil, fmt.Errorf("failed to add host: %w", err)
}
Expand Down Expand Up @@ -143,10 +139,7 @@ func (m *rtcdClientManager) hostsChecker() {

// we look for newly advertised hosts we may not have a client for yet.
for ip := range ipsMap {
m.mut.RLock()
_, ok := m.hosts[ip]
m.mut.RUnlock()
if !ok {
if h := m.getHost(ip); h == nil {
// create new client

// We add some jitter to try and avoid multiple clients to attempt
Expand All @@ -160,11 +153,6 @@ func (m *rtcdClientManager) hostsChecker() {
continue
}

if err := m.versionCheck(client); err != nil {
m.ctx.LogError(fmt.Sprintf("version compatibility check failed: %s", err.Error()), "host", ip)
continue
}

if err := m.addHost(ip, client); err != nil {
m.ctx.LogError(fmt.Sprintf("failed to add host: %s", err.Error()), "host", ip)
continue
Expand Down Expand Up @@ -220,6 +208,12 @@ func (m *rtcdClientManager) addHost(host string, client *rtcd.Client) (err error
return nil
}

func (m *rtcdClientManager) getHost(ip string) *rtcdHost {
m.mut.RLock()
defer m.mut.RUnlock()
return m.hosts[ip]
}

// GetHostForNewCall returns the host to which a new call should be routed.
// It performs a simple round-robin strategy based on number of calls.
// New calls are routed to the non-flagged host with the smaller count.
Expand Down Expand Up @@ -264,12 +258,9 @@ func (m *rtcdClientManager) Send(msg rtcd.ClientMessage, callID string) error {
}
host := state.Call.RTCDHost

m.mut.RLock()
h := m.hosts[host]
m.mut.RUnlock()

var client *rtcd.Client
if h == nil {
if h := m.getHost(host); h == nil {
m.ctx.LogDebug("creating client for missing host on send", "host", host)
client, err = m.newRTCDClient(m.rtcdURL, host, getDialFn(host, m.rtcdPort))
if err != nil {
return fmt.Errorf("failed to create new client: %w", err)
Expand Down Expand Up @@ -332,7 +323,8 @@ func (m *rtcdClientManager) versionCheck(client *rtcd.Client) error {
}

// Always support dev builds.
if info.BuildVersion == "" || strings.HasPrefix(info.BuildVersion, "dev") {
if info.BuildVersion == "" || info.BuildVersion == "master" || strings.HasPrefix(info.BuildVersion, "dev") {
m.ctx.LogInfo("skipping version compatibility check", "buildVersion", info.BuildVersion)
return nil
}

Expand Down Expand Up @@ -365,6 +357,18 @@ func (m *rtcdClientManager) newRTCDClient(rtcdURL, host string, dialFn rtcd.Dial
return fmt.Errorf("max reconnection attempts reached, removing client")
}

h := m.getHost(host)
if h == nil {
return fmt.Errorf("host is missing")
}

if h.isFlagged() {
if err := m.removeHost(host); err != nil {
m.ctx.LogError("failed to remove rtcd client: %w", err)
}
return fmt.Errorf("host was flagged")
}

// On disconnect, it's possible the rtcd server restarted
// and cleared its stored credentials, so we attempt to connect and
// register again if that fails.
Expand Down Expand Up @@ -399,6 +403,13 @@ func (m *rtcdClientManager) newRTCDClient(rtcdURL, host string, dialFn rtcd.Dial
return nil, err
}

if err := m.versionCheck(client); err != nil {
if err := client.Close(); err != nil {
m.ctx.LogError(fmt.Sprintf("failed to close client: %s", err.Error()))
}
return nil, fmt.Errorf("version compatibility check failed: %w", err)
}

return client, nil
}

Expand Down Expand Up @@ -641,3 +652,9 @@ func (m *rtcdClientManager) clientReader(client *rtcd.Client) {
}
}
}

func (h *rtcdHost) isFlagged() bool {
h.mut.RLock()
defer h.mut.RUnlock()
return h.flagged
}

0 comments on commit 1c423ed

Please sign in to comment.