Merge ~mpontillo/maas:fix-websocket-race-condition--bug-1802390 into maas:master

Proposed by Mike Pontillo
Status: Merged
Approved by: Mike Pontillo
Approved revision: 4ddca9e4b28c12709a88784dc35805acf0e4f780
Merge reported by: MAAS Lander
Merged at revision: not available
Proposed branch: ~mpontillo/maas:fix-websocket-race-condition--bug-1802390
Merge into: maas:master
Diff against target: 50 lines (+11/-3)
2 files modified
src/maasserver/websockets/protocol.py (+6/-2)
src/maasserver/websockets/tests/test_protocol.py (+5/-1)
Reviewer Review Type Date Requested Status
Newell Jensen (community) Approve
MAAS Lander unittests Pending
Review via email: mp+358527@code.launchpad.net

Commit message

LP: #1802390 - Fix race condition during websocket reconnect.

To post a comment you must log in.
Revision history for this message
Newell Jensen (newell-jensen) wrote :

Looks good code wise. Thanks for catching this!

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/src/maasserver/websockets/protocol.py b/src/maasserver/websockets/protocol.py
2index d92afef..f69e656 100644
3--- a/src/maasserver/websockets/protocol.py
4+++ b/src/maasserver/websockets/protocol.py
5@@ -93,6 +93,7 @@ class WebSocketProtocol(Protocol):
6 def __init__(self):
7 self.messages = deque()
8 self.user = None
9+ self.request = None
10 self.cache = {}
11
12 def connectionMade(self):
13@@ -119,8 +120,6 @@ class WebSocketProtocol(Protocol):
14 # This user is a keeper. Record it and process any message
15 # that have already been received.
16 self.user = user
17- self.processMessages()
18- self.factory.clients.append(self)
19
20 # Create the request for the handlers for this connection.
21 self.request = HttpRequest()
22@@ -148,6 +147,11 @@ class WebSocketProtocol(Protocol):
23 else:
24 self.request.META['SERVER_PORT'] = 5248
25
26+ # Be sure to process messages after the metadata is populated,
27+ # in order to avoid bug #1802390.
28+ self.processMessages()
29+ self.factory.clients.append(self)
30+
31 d.addCallback(authenticated)
32
33 def connectionLost(self, reason):
34diff --git a/src/maasserver/websockets/tests/test_protocol.py b/src/maasserver/websockets/tests/test_protocol.py
35index 4add31b..3d953ba 100644
36--- a/src/maasserver/websockets/tests/test_protocol.py
37+++ b/src/maasserver/websockets/tests/test_protocol.py
38@@ -106,7 +106,11 @@ class TestWebSocketProtocol(MAASTransactionServerTestCase):
39 def test_connectionMade_sets_the_request(self):
40 protocol, factory = self.make_protocol(patch_authenticate=False)
41 self.patch_autospec(protocol, "authenticate")
42- self.patch_autospec(protocol, "processMessages")
43+ # Be sure the request field is populated by the time that
44+ # processMessages() is called.
45+ processMessages_mock = self.patch_autospec(protocol, "processMessages")
46+ processMessages_mock.side_effect = lambda: self.assertThat(
47+ protocol.request.user, Equals(protocol.user))
48 protocol.authenticate.return_value = defer.succeed(sentinel.user)
49 protocol.connectionMade()
50 self.addCleanup(protocol.connectionLost, "")

Subscribers

People subscribed via source and target branches