From 86f9051a3060088f82ffc26caa0798c1a2880a57 Mon Sep 17 00:00:00 2001 From: Zoltan Papp Date: Mon, 3 Apr 2023 16:59:13 +0200 Subject: [PATCH] Fix/connection listener (#777) Fix add/remove connection listener In case we call the RemoveConnListener from Java then we lose the reference from the original instance --- client/android/client.go | 10 ++-- client/internal/peer/notifier.go | 67 ++++++++++++++------------- client/internal/peer/notifier_test.go | 66 ++++++++++++++++++++++++++ client/internal/peer/status.go | 12 ++--- 4 files changed, 111 insertions(+), 44 deletions(-) diff --git a/client/android/client.go b/client/android/client.go index 5e3c0c85a..3a7c2c8dc 100644 --- a/client/android/client.go +++ b/client/android/client.go @@ -118,12 +118,12 @@ func (c *Client) PeersList() *PeerInfoArray { return &PeerInfoArray{items: peerInfos} } -// AddConnectionListener add new network connection listener -func (c *Client) AddConnectionListener(listener ConnectionListener) { - c.recorder.AddConnectionListener(listener) +// SetConnectionListener set the network connection listener +func (c *Client) SetConnectionListener(listener ConnectionListener) { + c.recorder.SetConnectionListener(listener) } // RemoveConnectionListener remove connection listener -func (c *Client) RemoveConnectionListener(listener ConnectionListener) { - c.recorder.RemoveConnectionListener(listener) +func (c *Client) RemoveConnectionListener() { + c.recorder.RemoveConnectionListener() } diff --git a/client/internal/peer/notifier.go b/client/internal/peer/notifier.go index 4e618d2f8..b2d324c6c 100644 --- a/client/internal/peer/notifier.go +++ b/client/internal/peer/notifier.go @@ -14,32 +14,31 @@ const ( type notifier struct { serverStateLock sync.Mutex listenersLock sync.Mutex - listeners map[Listener]struct{} + listener Listener currentServerState bool currentClientState bool lastNotification int } func newNotifier() *notifier { - return ¬ifier{ - listeners: make(map[Listener]struct{}), - } + return ¬ifier{} } -func (n *notifier) addListener(listener Listener) { +func (n *notifier) setListener(listener Listener) { n.listenersLock.Lock() defer n.listenersLock.Unlock() n.serverStateLock.Lock() - go n.notifyListener(listener, n.lastNotification) + n.notifyListener(listener, n.lastNotification) n.serverStateLock.Unlock() - n.listeners[listener] = struct{}{} + + n.listener = listener } -func (n *notifier) removeListener(listener Listener) { +func (n *notifier) removeListener() { n.listenersLock.Lock() defer n.listenersLock.Unlock() - delete(n.listeners, listener) + n.listener = nil } func (n *notifier) updateServerStates(mgmState bool, signalState bool) { @@ -64,7 +63,7 @@ func (n *notifier) updateServerStates(mgmState bool, signalState bool) { } n.lastNotification = n.calculateState(newState, n.currentClientState) - go n.notifyAll(n.lastNotification) + n.notify(n.lastNotification) } func (n *notifier) clientStart() { @@ -72,7 +71,7 @@ func (n *notifier) clientStart() { defer n.serverStateLock.Unlock() n.currentClientState = true n.lastNotification = n.calculateState(n.currentServerState, true) - go n.notifyAll(n.lastNotification) + n.notify(n.lastNotification) } func (n *notifier) clientStop() { @@ -80,7 +79,7 @@ func (n *notifier) clientStop() { defer n.serverStateLock.Unlock() n.currentClientState = false n.lastNotification = n.calculateState(n.currentServerState, false) - go n.notifyAll(n.lastNotification) + n.notify(n.lastNotification) } func (n *notifier) clientTearDown() { @@ -88,33 +87,35 @@ func (n *notifier) clientTearDown() { defer n.serverStateLock.Unlock() n.currentClientState = false n.lastNotification = stateDisconnecting - go n.notifyAll(n.lastNotification) + n.notify(n.lastNotification) } func (n *notifier) isServerStateChanged(newState bool) bool { return n.currentServerState != newState } -func (n *notifier) notifyAll(state int) { +func (n *notifier) notify(state int) { n.listenersLock.Lock() defer n.listenersLock.Unlock() - - for l := range n.listeners { - n.notifyListener(l, state) + if n.listener == nil { + return } + n.notifyListener(n.listener, state) } func (n *notifier) notifyListener(l Listener, state int) { - switch state { - case stateDisconnected: - l.OnDisconnected() - case stateConnected: - l.OnConnected() - case stateConnecting: - l.OnConnecting() - case stateDisconnecting: - l.OnDisconnecting() - } + go func() { + switch state { + case stateDisconnected: + l.OnDisconnected() + case stateConnected: + l.OnConnected() + case stateConnecting: + l.OnConnecting() + case stateDisconnecting: + l.OnDisconnecting() + } + }() } func (n *notifier) calculateState(serverState bool, clientState bool) int { @@ -132,17 +133,17 @@ func (n *notifier) calculateState(serverState bool, clientState bool) int { func (n *notifier) peerListChanged(numOfPeers int) { n.listenersLock.Lock() defer n.listenersLock.Unlock() - - for l := range n.listeners { - l.OnPeersListChanged(numOfPeers) + if n.listener == nil { + return } + n.listener.OnPeersListChanged(numOfPeers) } func (n *notifier) localAddressChanged(fqdn, address string) { n.listenersLock.Lock() defer n.listenersLock.Unlock() - - for l := range n.listeners { - l.OnAddressChanged(fqdn, address) + if n.listener == nil { + return } + n.listener.OnAddressChanged(fqdn, address) } diff --git a/client/internal/peer/notifier_test.go b/client/internal/peer/notifier_test.go index f21193e06..a9045ac34 100644 --- a/client/internal/peer/notifier_test.go +++ b/client/internal/peer/notifier_test.go @@ -1,9 +1,48 @@ package peer import ( + "sync" "testing" ) +type mocListener struct { + lastState int + wg sync.WaitGroup + peers int +} + +func (l *mocListener) OnConnected() { + l.lastState = stateConnected + l.wg.Done() +} +func (l *mocListener) OnDisconnected() { + l.lastState = stateDisconnected + l.wg.Done() +} +func (l *mocListener) OnConnecting() { + l.lastState = stateConnecting + l.wg.Done() +} +func (l *mocListener) OnDisconnecting() { + l.lastState = stateDisconnecting + l.wg.Done() +} + +func (l *mocListener) OnAddressChanged(host, addr string) { + +} +func (l *mocListener) OnPeersListChanged(size int) { + l.peers = size +} + +func (l *mocListener) setWaiter() { + l.wg.Add(1) +} + +func (l *mocListener) wait() { + l.wg.Wait() +} + func Test_notifier_serverState(t *testing.T) { type scenario struct { @@ -30,3 +69,30 @@ func Test_notifier_serverState(t *testing.T) { }) } } + +func Test_notifier_SetListener(t *testing.T) { + listener := &mocListener{} + listener.setWaiter() + + n := newNotifier() + n.lastNotification = stateConnecting + n.setListener(listener) + listener.wait() + if listener.lastState != n.lastNotification { + t.Errorf("invalid state: %d, expected: %d", listener.lastState, n.lastNotification) + } +} + +func Test_notifier_RemoveListener(t *testing.T) { + listener := &mocListener{} + listener.setWaiter() + n := newNotifier() + n.lastNotification = stateConnecting + n.setListener(listener) + n.removeListener() + n.peerListChanged(1) + + if listener.peers != 0 { + t.Errorf("invalid state: %d", listener.peers) + } +} diff --git a/client/internal/peer/status.go b/client/internal/peer/status.go index 62841d6fc..508131816 100644 --- a/client/internal/peer/status.go +++ b/client/internal/peer/status.go @@ -293,14 +293,14 @@ func (d *Status) ClientTeardown() { d.notifier.clientTearDown() } -// AddConnectionListener add a listener to the notifier -func (d *Status) AddConnectionListener(listener Listener) { - d.notifier.addListener(listener) +// SetConnectionListener set a listener to the notifier +func (d *Status) SetConnectionListener(listener Listener) { + d.notifier.setListener(listener) } -// RemoveConnectionListener remove a listener from the notifier -func (d *Status) RemoveConnectionListener(listener Listener) { - d.notifier.removeListener(listener) +// RemoveConnectionListener remove the listener from the notifier +func (d *Status) RemoveConnectionListener() { + d.notifier.removeListener() } func (d *Status) onConnectionChanged() {