Reduce impact of org.gnome.SessionManager slow-down / freeze (issue 2…

…77) (#282)

Previously: Using the org.gnome.SessionManager method repeatedly would
freeze system in 400-600 iterations.

Now: Get to ~2000 iterations before system freezes. 

Contents
--------
* Dbus connections are now closed. In addition, gc.collect() is called
  each time connection is closed (at least for now), as adding it helped
  to get more iteration cycles.
* Add warning to docs of org.gnome.SessionManager
* bonus: Some refactoring of dbus adapter tests + remove the temporary
  warning ignore introduced in #278.

docs/source/methods-reference.md

@@ -26,6 +26,12 @@ Methods are different ways of entering/keeping in a Mode. A Method may support o
 - **Requirements**: D-Bus, GNOME Desktop Environment with gnome-session running. The exact version of required GNOME is unknown, but this should work from GNOME 2.24 ([2008-09-23](https://gitlab.gnome.org/GNOME/gnome-session/-/tags/GNOME_SESSION_2_24_0)) onwards. See [version history of org.gnome.SessionManager.xml](https://gitlab.gnome.org/GNOME/gnome-session/-/commits/main/gnome-session/org.gnome.SessionManager.xml). At least [this](https://fedoraproject.org/wiki/Desktop/Whiteboards/InhibitApis) and [this](https://bugzilla.redhat.com/show_bug.cgi?id=529287#c3) mention GNOME 2.24.
 - **Tested on**:  Ubuntu 22.04.3 LTS with GNOME 42.9 ([PR #138](https://github.com/fohrloop/wakepy/pull/138) by [fohrloop](https://github.com/fohrloop/)).
 
+````{admonition} May slow down system if called repeatedly
+:class: warning
+
+If used hundreds or thousands of times, may slow down system. See: [wakepy/#277](https://github.com/fohrloop/wakepy/issues/277)
+````
+
 (keep-running-windows-stes)=
 ### SetThreadExecutionState
 
@@ -34,7 +40,6 @@ Methods are different ways of entering/keeping in a Mode. A Method may support o
 
 Since this method prevents sleep, screen can be only locked automatically if a screen saver is enabled and it set to ask for password. See [this](#checking-if-windows-will-lock-screen-automatically) for details.
 
-
 ````
 
 - **Name**: `SetThreadExecutionState`
@@ -106,6 +111,12 @@ print('SPI_GETSCREENSAVETIMEOUT', retval.value)
 - **Requirements**: Same as [keep.running](#keep-running-org-gnome-sessionmanager) using org.gnome.SessionManager.
 - **Tested on**:  Ubuntu 22.04.3 LTS with GNOME 42.9 ([PR #138](https://github.com/fohrloop/wakepy/pull/138) by [fohrloop](https://github.com/fohrloop/)).
 
+````{admonition} May slow down system if called repeatedly
+:class: warning
+
+If used hundreds or thousands of times, may slow down system. See: [wakepy/#277](https://github.com/fohrloop/wakepy/issues/277)
+````
+
 (keep-presenting-org-freedesktop-screensaver)=
 ### org.freedesktop.ScreenSaver
 - **Name**: `org.freedesktop.ScreenSaver`

src/wakepy/core/dbus.py

@@ -15,6 +15,7 @@
 
 from __future__ import annotations
 
+import gc
 import typing
 from typing import Any, Dict, List, NamedTuple, Optional, Tuple, Type, Union
 
@@ -317,6 +318,26 @@ def _create_new_connection(
         """
         raise NotImplementedError("Implement in subclass")
 
+    def close_connections(self) -> None:
+        """Close all the connections open in this adapter."""
+
+        for bus in list(self._connections):
+            connection = self._connections.pop(bus)
+            self.close_connection(connection)
+            del connection
+
+            # The gc collect here frees up some resources but unfortunately
+            # takes some time. Tried to call this only every 50th time but
+            # it still makes Gnome freeze if activating and deactivating the
+            # keepawake repeatedly. This is a bit ugly but it's required until
+            # there's a better solution.
+            # See: https://github.com/fohrloop/wakepy/issues/277
+            gc.collect()
+
+    def close_connection(self, connection: object) -> None:
+        """Close a dbus connection. Implement in a subclass"""
+        raise NotImplementedError("Implement in subclass")
+
 
 def get_dbus_adapter(
     dbus_adapter: Optional[Type[DBusAdapter] | DBusAdapterTypeSeq] = None,

src/wakepy/core/method.py

@@ -368,6 +368,9 @@ def deactivate_method(method: Method, heartbeat: Optional[Heartbeat] = None) ->
             "clearing the mode. "
         )
 
+    if method.dbus_adapter:
+        method.dbus_adapter.close_connections()
+
 
 def get_platform_supported(method: Method, platform: PlatformName) -> bool:
     """Checks if method is supported by the platform

src/wakepy/dbus_adapters/jeepney.py

@@ -1,3 +1,5 @@
+# mypy: disable-error-code="no-any-unimported"
+
 from __future__ import annotations
 
 import typing
@@ -42,12 +44,12 @@ def process(self, call: DBusMethodCall) -> object:
             body=call.args,
         )
 
-        connection = cast(DBusConnection, self._get_connection(call.method.bus))  # type: ignore[no-any-unimported]
+        connection = cast(DBusConnection, self._get_connection(call.method.bus))
         reply = connection.send_and_get_reply(msg, timeout=self.timeout)
         resp = unwrap_msg(reply)
         return resp
 
-    def _create_new_connection(  # type: ignore[no-any-unimported]
+    def _create_new_connection(
         self, bus: TypeOfBus = BusType.SESSION
     ) -> DBusConnection:
         try:
@@ -64,3 +66,6 @@ def _create_new_connection(  # type: ignore[no-any-unimported]
                 ) from exc
             else:
                 raise
+
+    def close_connection(self, connection: DBusConnection) -> None:
+        connection.close()

tests/integration/conftest.py

@@ -6,7 +6,6 @@
 import logging
 import subprocess
 import sys
-import warnings
 
 import pytest
 
@@ -33,10 +32,7 @@ def gc_collect_after_dbus_integration_tests():
     # this as garbage colletion is triggered also automatically. The garbage
     # collection must be triggered here manually as the warnings are
     # ResourceWarning is only filtered away in the dbus integration tests.
-
-    with warnings.catch_warnings():
-        warnings.simplefilter("ignore")
-        gc.collect()
+    gc.collect()
 
     logger.debug("called gc.collect")
 

tests/integration/test_dbus_adapters.py

@@ -32,108 +32,102 @@
 
 
 @pytest.mark.usefixtures("dbus_calculator_service")
-def test_jeepney_dbus_adapter_numberadd(numberadd_method):
-    adapter = JeepneyDBusAdapter()
-    call = DBusMethodCall(numberadd_method, (2, 3))
-    assert adapter.process(call) == (5,)
-
-
-@pytest.mark.usefixtures("dbus_calculator_service")
-def test_jeepney_dbus_adapter_number_multiply(numbermultiply_method):
-    adapter = JeepneyDBusAdapter()
-    call = DBusMethodCall(numbermultiply_method, (-5, 3))
-    assert adapter.process(call) == (-15,)
-
-
-@pytest.mark.usefixtures("dbus_calculator_service")
-def test_jeepney_dbus_adapter_wrong_arguments(numbermultiply_method):
-    adapter = JeepneyDBusAdapter()
-    with pytest.raises(struct.error, match="required argument is not an intege"):
-        # Bad input arg type
-        adapter.process(DBusMethodCall(numbermultiply_method, (-5, "foo")))
-    with pytest.raises(
-        ValueError, match=re.escape("Expected args to have 2 items! (has: 3)")
-    ):
-        # Too many input args
-        adapter.process(DBusMethodCall(numbermultiply_method, (-5, 3, 3)))
-
-
-@pytest.mark.usefixtures("dbus_calculator_service")
-def test_jeepney_dbus_adapter_wrong_signature(calculator_service_addr):
-    wrong_method = DBusMethod(
-        name="SimpleNumberMultiply",
-        signature="is",  # this is wrong: should be "ii"
-        params=("first_number", "second_number"),
-        output_signature="i",
-        output_params=("result",),
-    ).of(calculator_service_addr)
+class TestJeepneyCalculatorService:
+
+    def test_numberadd(self, numberadd_method):
+        adapter = JeepneyDBusAdapter()
+        call = DBusMethodCall(numberadd_method, (2, 3))
+        assert adapter.process(call) == (5,)
+
+    def test_number_multiply(self, numbermultiply_method):
+        adapter = JeepneyDBusAdapter()
+        call = DBusMethodCall(numbermultiply_method, (-5, 3))
+        assert adapter.process(call) == (-15,)
+
+    def test_multiply_with_wrong_arguments(self, numbermultiply_method):
+        adapter = JeepneyDBusAdapter()
+        with pytest.raises(struct.error, match="required argument is not an intege"):
+            # Bad input arg type
+            adapter.process(DBusMethodCall(numbermultiply_method, (-5, "foo")))
+        with pytest.raises(
+            ValueError, match=re.escape("Expected args to have 2 items! (has: 3)")
+        ):
+            # Too many input args
+            adapter.process(DBusMethodCall(numbermultiply_method, (-5, 3, 3)))
+
+    def test_wrong_signature(self, calculator_service_addr):
+        wrong_method = DBusMethod(
+            name="SimpleNumberMultiply",
+            signature="is",  # this is wrong: should be "ii"
+            params=("first_number", "second_number"),
+            output_signature="i",
+            output_params=("result",),
+        ).of(calculator_service_addr)
+
+        adapter = JeepneyDBusAdapter()
+        with pytest.raises(
+            jeepney.wrappers.DBusErrorResponse,
+            match="org.github.wakepy.TestCalculatorService.Error.OtherError",
+        ):
+            # The args are correct for the DBusMethod, but the DBusMethod is
+            # not correct for the service (signature is wrong)
+            adapter.process(DBusMethodCall(wrong_method, (-5, "sdaasd")))
+
+    def test_nonexisting_method(self, calculator_service_addr):
+        non_existing_method = DBusMethod(
+            name="ThisDoesNotExist",
+            signature="ii",
+            params=("first_number", "second_number"),
+            output_signature="i",
+            output_params=("result",),
+        ).of(calculator_service_addr)
+        adapter = JeepneyDBusAdapter()
+
+        with pytest.raises(
+            jeepney.wrappers.DBusErrorResponse,
+            match="org.github.wakepy.TestCalculatorService.Error.NoMethod",
+        ):
+            # No such method: ThisDoesNotExist
+            adapter.process(DBusMethodCall(non_existing_method, (-5, 2)))
+
+    def test_wrong_service_definition(self, private_bus: str):
+        adapter = JeepneyDBusAdapter()
+
+        wrong_service_addr = DBusAddress(
+            bus=private_bus,
+            service="org.github.wakepy.WrongService",
+            path="/org/github/wakepy/TestCalculatorService",
+            interface="org.github.wakepy.TestCalculatorService",
+        )
+        wrong_method = DBusMethod(
+            name="SimpleNumberMultiply",
+            signature="ii",
+            params=("first_number", "second_number"),
+            output_signature="i",
+            output_params=("result",),
+        ).of(wrong_service_addr)
 
-    adapter = JeepneyDBusAdapter()
-    with pytest.raises(
-        jeepney.wrappers.DBusErrorResponse,
-        match="org.github.wakepy.TestCalculatorService.Error.OtherError",
-    ):
         # The args are correct for the DBusMethod, but the DBusMethod is not
         # correct for the service (signature is wrong)
-        adapter.process(DBusMethodCall(wrong_method, (-5, "sdaasd")))
-
-
-@pytest.mark.usefixtures("dbus_calculator_service")
-def test_jeepney_dbus_adapter_nonexisting_method(calculator_service_addr):
-    non_existing_method = DBusMethod(
-        name="ThisDoesNotExist",
-        signature="ii",
-        params=("first_number", "second_number"),
-        output_signature="i",
-        output_params=("result",),
-    ).of(calculator_service_addr)
-    adapter = JeepneyDBusAdapter()
-
-    with pytest.raises(
-        jeepney.wrappers.DBusErrorResponse,
-        match="org.github.wakepy.TestCalculatorService.Error.NoMethod",
-    ):
-        # No such method: ThisDoesNotExist
-        adapter.process(DBusMethodCall(non_existing_method, (-5, 2)))
-
-
-@pytest.mark.usefixtures("dbus_calculator_service")
-def test_jeepney_dbus_adapter_wrong_service_definition(private_bus: str):
-    adapter = JeepneyDBusAdapter()
-
-    wrong_service_addr = DBusAddress(
-        bus=private_bus,
-        service="org.github.wakepy.WrongService",
-        path="/org/github/wakepy/TestCalculatorService",
-        interface="org.github.wakepy.TestCalculatorService",
-    )
-    wrong_method = DBusMethod(
-        name="SimpleNumberMultiply",
-        signature="ii",
-        params=("first_number", "second_number"),
-        output_signature="i",
-        output_params=("result",),
-    ).of(wrong_service_addr)
-
-    # The args are correct for the DBusMethod, but the DBusMethod is not
-    # correct for the service (signature is wrong)
-    with pytest.raises(
-        jeepney.wrappers.DBusErrorResponse,
-        match=re.escape(
-            "The name org.github.wakepy.WrongService was not provided by any .service "
-            "files"
-        ),
-    ):
-        adapter.process(DBusMethodCall(wrong_method, (-5, 2)))
+        with pytest.raises(
+            jeepney.wrappers.DBusErrorResponse,
+            match=re.escape(
+                "The name org.github.wakepy.WrongService was not provided by any "
+                ".service files"
+            ),
+        ):
+            adapter.process(DBusMethodCall(wrong_method, (-5, 2)))
 
 
 @pytest.mark.usefixtures("dbus_string_operation_service")
-def test_jeepney_dbus_adapter_string_shorten(string_shorten_method):
-    # The service shortens a string to a given number of characters and tells
-    # how many characters were dropped out.
-    adapter = JeepneyDBusAdapter()
-    call = DBusMethodCall(string_shorten_method, ("cat pinky", 3))
-    assert adapter.process(call) == ("cat", 6)
+class TestJeepneyStringOperationService:
+
+    def test_jeepney_dbus_adapter_string_shorten(self, string_shorten_method):
+        # The service shortens a string to a given number of characters and
+        # tells how many characters were dropped out.
+        adapter = JeepneyDBusAdapter()
+        call = DBusMethodCall(string_shorten_method, ("cat pinky", 3))
+        assert adapter.process(call) == ("cat", 6)
 
 
 class TestFailuresOnConnectionCreation:
@@ -182,9 +176,20 @@ def test_random_keyerror_on_connection_creation(
                 self.adapter.process(self.call)
 
 
-def test_jeepney_adapter_caching(private_bus: str):
-    adapter = JeepneyDBusAdapter()
-    con = adapter._get_connection(private_bus)
+class TestJeepneyDbusAdapter:
+
+    def test_close_connections(self, private_bus: str):
+        adapter = JeepneyDBusAdapter()
+        con = adapter._get_connection(private_bus)
+        # There seems to be no other way checking that the connection is
+        # active..?
+        assert not con.sock._closed  # type: ignore[attr-defined]
+        adapter.close_connections()
+        assert con.sock._closed  # type: ignore[attr-defined]
+
+    def test_adapter_caching(self, private_bus: str):
+        adapter = JeepneyDBusAdapter()
+        con = adapter._get_connection(private_bus)
 
-    # Call again with same bus name -> get same (cached) connection.
-    assert adapter._get_connection(private_bus) is con
+        # Call again with same bus name -> get same (cached) connection.
+        assert adapter._get_connection(private_bus) is con