NM + Network: Cleanup network state updates

- Use the pygobject signals to properly check whether we have the
correct UUID
- Delete polling every second as that is hopefully not useful any more
- Delete unknown state retry mechanism for the same reason
- Should fix #464 and hopefully #492

eduvpn/app.py

@@ -7,7 +7,7 @@
 from .variants import ApplicationVariant
 from .state_machine import StateMachine, InvalidStateTransition
 from .config import Configuration
-from .utils import run_in_background_thread, run_periodically, run_delayed, cancel_at_context_end
+from .utils import run_in_background_thread, run_delayed, cancel_at_context_end
 
 
 logger = logging.getLogger(__name__)
@@ -68,21 +68,7 @@ def on_network_update_callback(state):
             network.on_state_update_callback(self, state)
 
         from . import network
-        if not nm.subscribe_to_status_changes(on_network_update_callback):
-            logger.warning(
-                "unable to subscribe to network updates; "
-                "the application may not reflect the current state"
-            )
-
-        def check_network_state():
-            from .network import check_network_state
-            check_network_state(self)
-
-        run_periodically(
-            check_network_state,
-            CHECK_NETWORK_INTERVAL,
-            'check-network',
-        )
+        nm.subscribe_to_status_changes(on_network_update_callback)
 
     @run_in_background_thread('init-server-db')
     def initialize_server_db(self):

eduvpn/network.py

@@ -2,14 +2,13 @@
 import logging
 import enum
 from functools import partial
-from time import sleep
 from . import nm
 from . import settings
 from . import storage
 from .state_machine import BaseState
 from .app import Application
 from .server import ConfiguredServer as Server, Protocol
-from .utils import run_in_background_thread, translated_property
+from .utils import translated_property
 
 logger = logging.getLogger(__name__)
 
@@ -50,7 +49,7 @@ def set_disconnected(self, app: Application) -> 'NetworkState':
         return DisconnectedState()
 
     def set_unknown(self, app: Application) -> 'NetworkState':
-        return enter_unknown_state(app)
+        return UnknownState()
 
     def set_error(self, app: Application, message: Optional[str] = None) -> 'NetworkState':
         return ConnectionErrorState(message)
@@ -178,38 +177,6 @@ def reconnect(app: Application) -> NetworkState:
     return ReconnectingState()
 
 
-UNKNOWN_STATE_MAX_RETRIES = 5
-
-
-def enter_unknown_state(app: Application) -> NetworkState:
-    # Set the state temporarily to unknown but keep polling for updates,
-    # since we don't always get notified by the update callback.
-    @run_in_background_thread('poll-network-state')
-    def determine_network_state_thread():
-        counter = 0
-        state = nm.get_connection_state()
-        while state is nm.ConnectionState.UNKNOWN:
-            if counter > UNKNOWN_STATE_MAX_RETRIES:
-                # After a number of retries, assume we've disconnected
-                # so the user can try to connect again.
-                state = nm.ConnectionState.DISCONNECTED
-                logger.debug(
-                    "network state has been unknown for too long,"
-                    " fall back to disconnected state"
-                )
-                break
-            sleep(1)
-            counter += 1
-            if not isinstance(app.network_state, UnknownState):
-                return
-            state = nm.get_connection_state()
-            logger.debug(f"polling network state: {state}")
-        app.make_func_threadsafe(on_state_update_callback)(app, state)
-
-    determine_network_state_thread()
-    return UnknownState()
-
-
 def on_state_update_callback(app: Application, state: nm.ConnectionState):
     """
     Callback for whenever a connection state changes.
@@ -261,24 +228,11 @@ def get_corresponding_state(
     elif state is nm.ConnectionState.FAILED:
         return ConnectionErrorState(None)
     elif state is nm.ConnectionState.UNKNOWN:
-        return enter_unknown_state(app)
+        return UnknownState()
     else:
         raise ValueError(state)
 
 
-def check_network_state(app: Application):
-    state = nm.get_connection_state()
-    if state is not nm.ConnectionState.UNKNOWN:
-        network_state = get_corresponding_state(app, state)
-        if type(network_state) != type(app.network_state):  # NOQA
-            # If the state remains different for one second,
-            # update it in the app. This prevents
-            # confusing transitions around connect/disconnect.
-            sleep(1)
-            if nm.get_connection_state() is state:
-                app.make_func_threadsafe(on_state_update_callback)(app, state)
-
-
 class ConnectingState(NetworkState):
     """
     The network is currently trying to connect to a server.

eduvpn/nm.py

@@ -591,37 +591,43 @@ def get_dbus() -> Optional['dbus.SystemBus']:
 
 def subscribe_to_status_changes(
     callback: Callable[[ConnectionState], Any],
-) -> bool:
+):
     """
-    Subscribe to all network status changes via DBus.
+    Subscribe to network status changes via the NM client.
 
     The callback argument is called with the connection state and reason
     whenever they change.
-
-    False is returned on failure.
     """
-    bus = get_dbus()
-    if bus is None:
-        return False
 
-    def wrapped_callback_vpn(state_code: 'dbus.UInt32', reason_code: 'dbus.UInt32'):
-        state = NM.VpnConnectionState(state_code)
-        callback(ConnectionState.from_vpn_state(state))
+    # The callback to monitor state changes
+    # Let the state machine know for state updates
+    def wrapped_callback(active: 'NM.ActiveConnection', state_code: int, reason_code: int):
+        if active.get_uuid() != get_uuid():
+            return
 
-    def wrapped_callback_wg(state_code: 'dbus.UInt32', reason_code: 'dbus.UInt32'):
         state = NM.ActiveConnectionState(state_code)
         callback(ConnectionState.from_active_state(state))
 
-    bus.add_signal_receiver(
-        handler_function=wrapped_callback_vpn,
-        dbus_interface='org.freedesktop.NetworkManager.VPN.Connection',
-        signal_name='VpnStateChanged',
-    )
-    bus.add_signal_receiver(
-        handler_function=wrapped_callback_wg,
-        dbus_interface='org.freedesktop.NetworkManager.Connection.Active',
-        signal_name='StateChanged',
-    )
+    # Connect the state changed signal for an active connection
+    def connect(con: 'NM.ActiveConnection'):
+        con.connect("state-changed", wrapped_callback)
+
+    # The callback when a connection gets added
+    # Connect the signals
+    def wrapped_connection_added(client: 'NM.Client', active_con: 'NM.ActiveConnection'):
+        if active_con.get_uuid() != get_uuid():
+            return
+        connect(active_con)
+
+    # If a connection was found already then...
+    client = get_client()
+    active_con = get_active_connection()
+
+    if active_con:
+        connect(active_con)
+
+    # Connect the active connection added signal
+    client.connect("active-connection-added", wrapped_connection_added)
     return True