Make sure the Conn object only calls the notifyDisconnect callbacks

once, by replacing the current subscription object with a new one
after delivering the notifications. A second call could mess up the
newer connection's state. (#4678)

master/buildbot/test/unit/worker/test_protocols_base.py

@@ -43,3 +43,14 @@ def test_notify(self):
  self.assertEqual(cb.call_args_list, [])
  self.conn.notifyDisconnected()
  self.assertNotEqual(cb.call_args_list, [])
+
+ def test_notify_twice(self):
+ cb = mock.Mock()
+
+ self.conn.notifyOnDisconnect(cb)
+ self.assertEqual(cb.call_args_list, [])
+ self.conn.notifyDisconnected()
+ self.assertNotEqual(cb.call_args_list, [])
+ cb.call_args_list = []
+ self.conn.notifyDisconnected()
+ self.assertEqual(cb.call_args_list, [])

master/buildbot/worker/manager.py

@@ -157,6 +157,9 @@ def newConnection(self, conn, workerName):
  self.connections[workerName] = conn
 
  def remove():
+ assert (
+ self.connections.get(workerName, None) == conn
+ ), f"Attempt to remove non-connection entry for {workerName}"
  del self.connections[workerName]
 
  conn.notifyOnDisconnect(remove)

master/buildbot/worker/protocols/base.py

@@ -107,6 +107,8 @@ def notifyOnDisconnect(self, cb):
 
  def notifyDisconnected(self):
  self._disconnectSubs.deliver()
+ # Ensure that the disconnect notifications are not called twice
+ self._disconnectSubs = subscription.SubscriptionPoint(self._disconnectSubs.name)
 
  def loseConnection(self):
  raise NotImplementedError