[lxc-devel] [lxd/master] lxd/events: Fix race condition in event handlers

[stgraber on Github]

A non-text attachment was scrubbed...
Name: not available
Type: text/x-mailbox
Size: 579 bytes
Desc: not available
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20170906/6cb3f7e5/attachment.bin>
-------------- next part --------------
From 602b7292771fd1828a7b566868c2711446485624 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?St=C3=A9phane=20Graber?= <stgraber at ubuntu.com>
Date: Wed, 6 Sep 2017 01:10:22 -0400
Subject: [PATCH] lxd/events: Fix race condition in event handlers
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

This adds additional locking around the event handler code to ensure
that there is no race between the time where we first fail to send a
message and the time where the listener is removed from the list.

Closes #3770

Signed-off-by: Stéphane Graber <stgraber at ubuntu.com>
---
 lxd/events.go | 32 +++++++++++++++++++++-----------
 1 file changed, 21 insertions(+), 11 deletions(-)

diff --git a/lxd/events.go b/lxd/events.go
index df3bdc600..d7aed0422 100644
--- a/lxd/events.go
+++ b/lxd/events.go
@@ -51,7 +51,8 @@ type eventListener struct {
 	messageTypes []string
 	active       chan bool
 	id           string
-	msgLock      sync.Mutex
+	lock         sync.Mutex
+	done         bool
 }
 
 type eventsServe struct {
@@ -92,13 +93,6 @@ func eventsSocket(r *http.Request, w http.ResponseWriter) error {
 
 	<-listener.active
 
-	eventsLock.Lock()
-	delete(eventListeners, listener.id)
-	eventsLock.Unlock()
-
-	listener.connection.Close()
-	logger.Debugf("Disconnected events listener: %s", listener.id)
-
 	return nil
 }
 
@@ -127,16 +121,32 @@ func eventSend(eventType string, eventMessage interface{}) error {
 		}
 
 		go func(listener *eventListener, body []byte) {
+			// Check that the listener still exists
 			if listener == nil {
 				return
 			}
 
-			listener.msgLock.Lock()
-			err = listener.connection.WriteMessage(websocket.TextMessage, body)
-			listener.msgLock.Unlock()
+			// Ensure there is only a single even going out at the time
+			listener.lock.Lock()
+			defer listener.lock.Unlock()
 
+			// Make sure we're not done already
+			if listener.done {
+				return
+			}
+
+			err = listener.connection.WriteMessage(websocket.TextMessage, body)
 			if err != nil {
+				// Remove the listener from the list
+				eventsLock.Lock()
+				delete(eventListeners, listener.id)
+				eventsLock.Unlock()
+
+				// Disconnect the listener
+				listener.connection.Close()
 				listener.active <- false
+				listener.done = true
+				logger.Debugf("Disconnected events listener: %s", listener.id)
 			}
 		}(listener, body)
 	}