From 4ad3a9ee8ff69092f29313d2d5e2c9c9ad5830ea Mon Sep 17 00:00:00 2001
From: Sunil Mohan Adapa <sunil@medhas.org>
Date: Tue, 7 Jan 2020 22:25:27 -0800
Subject: [PATCH] networks: Fix crashing when accessing network manager D-Bus
 API

Network Manager client fetches the basic information from Network Manager,
caches it and updates the cache whenever it receives the signal. So, create only
a single instance of it and reuse it. Reusing from different threads is
apparently fine because the underlying DBusConnection is a singleton[1] that is
meant to be used from multiple threads.

The glib main loop context that is related to the client must run even after the
network manager client object goes away[2]. So, create the network manager
client instance from a thread that continues to run the glib main thread.

This fixes an infrequent crash when accessing network manager page. The problem
was reproducible from very early version of Network Manager and FreedomBox.
However, in more recent versions of NetworkManager 1.20 with recent DBus changes
in FreedomBox, the problem is more prominent. The problem reduces to a simple
warning in NetworkManager 1.22.

Closes: #1724
Closes: #382

Tests:

- Reproduce the problem by accessing the networks app index page repeatedly.

- Create a simple test case file to reproduce the problem and ensure that the
  fix works there.

- On Network Manager 1.20 repeatedly access the networks app index page and
  create/delete/activate/deactivate/show connections.

- On Network Manager 1.22 repeatedly access the networks app index page and
  create/delete/activate/deactivate/show connections.

Links:

1) https://developer.gnome.org/gio/unstable/GDBusConnection.html#g-bus-get-sync

2) https://developer.gnome.org/libnm/stable/NMClient.html#nm-client-get-main-context

Signed-off-by: Sunil Mohan Adapa <sunil@medhas.org>
Reviewed-by: James Valleroy <jvalleroy@mailbox.org>
---
 plinth/dbus.py               |  6 +++++-
 plinth/network.py            | 19 ++++++++++++++++++-
 plinth/tests/test_network.py | 29 +++++++++++++++++++++++++++++
 3 files changed, 52 insertions(+), 2 deletions(-)

diff --git a/plinth/dbus.py b/plinth/dbus.py
index 3b3ec2efa..eedc46f98 100755
--- a/plinth/dbus.py
+++ b/plinth/dbus.py
@@ -23,7 +23,7 @@ import threading
 
 from plinth.utils import import_from_gi
 
-from . import setup
+from . import network, setup
 
 glib = import_from_gi('GLib', '2.0')
 gio = import_from_gi('Gio', '2.0')
@@ -149,6 +149,10 @@ def _run():
     _server = DBusServer()
     _server.connect()
 
+    # Initialize all other modules that glib main loop
+    # XXX: Refactor this code into separate 'glib' module later
+    network.init()
+
     global _main_loop
     _main_loop = glib.MainLoop()
     _main_loop.run()
diff --git a/plinth/network.py b/plinth/network.py
index d6c1d0ba5..b061f7540 100644
--- a/plinth/network.py
+++ b/plinth/network.py
@@ -33,6 +33,8 @@ nm = import_from_gi('NM', '1.0')
 
 logger = logging.getLogger(__name__)
 
+_client = None
+
 CONNECTION_TYPE_NAMES = collections.OrderedDict([
     ('802-3-ethernet', _('Ethernet')),
     ('802-11-wireless', _('Wi-Fi')),
@@ -61,9 +63,24 @@ def ipv4_int_to_string(address_int):
     return socket.inet_ntoa(struct.pack('=I', address_int))
 
 
+def init():
+    """Create and keep a network manager client instance."""
+    def new_callback(source_object, result, user_data):
+        """Called when new() operation is complete."""
+        global _client
+        _client = nm.Client.new_finish(result)
+        logger.info('Created Network manager client')
+
+    logger.info('Creating network manager client')
+    nm.Client.new_async(None, new_callback, None)
+
+
 def get_nm_client():
     """Return a network manager client object."""
-    return nm.Client.new(None)
+    if _client:
+        return _client
+
+    raise Exception('Client not yet ready')
 
 
 def _callback(source_object, result, user_data):
diff --git a/plinth/tests/test_network.py b/plinth/tests/test_network.py
index e417f6967..56b60cf76 100644
--- a/plinth/tests/test_network.py
+++ b/plinth/tests/test_network.py
@@ -19,10 +19,13 @@ Test module for network configuration utilities.
 """
 
 import copy
+import threading
 import time
 
 import pytest
 
+from plinth.utils import import_from_gi
+
 ethernet_settings = {
     'common': {
         'type': '802-3-ethernet',
@@ -83,6 +86,32 @@ pppoe_settings = {
 }
 
 
+@pytest.fixture(autouse=True, scope='module')
+def fixture_network_module_init():
+    """Initialize network module in a separate thread."""
+    from plinth import network as network_module
+    glib = import_from_gi('GLib', '2.0')
+    main_loop = glib.MainLoop()
+
+    def main_loop_runner():
+        """Initialize the network module and run glib main loop until quit."""
+        network_module.init()
+        main_loop.run()
+
+    thread = threading.Thread(target=main_loop_runner)
+    thread.start()
+
+    while not network_module._client:
+        time.sleep(0.1)
+
+    yield
+
+    if main_loop:
+        main_loop.quit()
+
+    thread.join()
+
+
 @pytest.fixture(name='network')
 def fixture_network(needs_root):
     """Return the network module. Load it conservatively."""