Skip to content

Commit

Permalink
Merge pull request #8057 from howbazaar/fix-connection-resource-leak
Browse files Browse the repository at this point in the history
Don't release resources until all current requests are complete.

If a websocket connection is closed while the login is in process, the rpc package was releasing the apiHandler resources before all the current requests were finished. If one of those requests would add resources, like an agent pinger, these would be leaked.

This branch changes the connection code to wait for all current requests to be finished before it kills the root object (which calls StopAll on the resources).

## Bug reference

Other part of the fix for https://bugs.launchpad.net/juju/+bug/1731745
  • Loading branch information
jujubot committed Nov 14, 2017
2 parents 4b75a1d + 03a697a commit eb64093
Showing 1 changed file with 12 additions and 0 deletions.
12 changes: 12 additions & 0 deletions rpc/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -284,6 +284,9 @@ func (conn *Conn) Close() error {
}
conn.closing = true
if conn.root != nil {
// Kill calls down into the resources to stop all the resources which
// includes watchers. The watches need to be killed in order for their
// API methods to return, otherwise they are just waiting.
conn.root.Kill()
}
conn.mutex.Unlock()
Expand All @@ -292,6 +295,15 @@ func (conn *Conn) Close() error {
// and write their replies before closing the codec.
conn.srvPending.Wait()

conn.mutex.Lock()
if conn.root != nil {
// It is possible that since we last Killed the root, other resources
// may have been added during some of the pending call resoulutions.
// So to release these resources, double tap the root.
conn.root.Kill()
}
conn.mutex.Unlock()

// Closing the codec should cause the input loop to terminate.
if err := conn.codec.Close(); err != nil {
logger.Debugf("error closing codec: %v", err)
Expand Down

0 comments on commit eb64093

Please sign in to comment.