From 78c3e24beb6cee4f69ba3a4903de3a24cb1a28d6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Niko=20F=C3=B6hr?= Date: Thu, 9 May 2024 23:19:38 +0300 Subject: [PATCH 1/7] add: close_connections to DBusAdapter --- src/wakepy/core/dbus.py | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/wakepy/core/dbus.py b/src/wakepy/core/dbus.py index 06c1a9f6..5786f586 100644 --- a/src/wakepy/core/dbus.py +++ b/src/wakepy/core/dbus.py @@ -317,6 +317,15 @@ def _create_new_connection( """ raise NotImplementedError("Implement in subclass") + def close_connections(self): + """Close all the connections open in this adapter.""" + for connection in self._connections.values(): + self.close_connection(connection) + + def close_connection(self, connection: object): + """Close a dbus connection. Implement in a subclass""" + raise NotImplementedError("Implement in subclass") + def get_dbus_adapter( dbus_adapter: Optional[Type[DBusAdapter] | DBusAdapterTypeSeq] = None, From 2d44415dc1e2b27f536e77b71d777e52eea94f84 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Niko=20F=C3=B6hr?= Date: Thu, 16 May 2024 22:02:50 +0300 Subject: [PATCH 2/7] close dbus connections when deactivating a mode --- src/wakepy/core/dbus.py | 14 +++++++++++++- src/wakepy/core/method.py | 3 +++ src/wakepy/dbus_adapters/jeepney.py | 3 +++ 3 files changed, 19 insertions(+), 1 deletion(-) diff --git a/src/wakepy/core/dbus.py b/src/wakepy/core/dbus.py index 5786f586..7f59dc5e 100644 --- a/src/wakepy/core/dbus.py +++ b/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 @@ -319,8 +320,19 @@ def _create_new_connection( def close_connections(self): """Close all the connections open in this adapter.""" - for connection in self._connections.values(): + + 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): """Close a dbus connection. Implement in a subclass""" diff --git a/src/wakepy/core/method.py b/src/wakepy/core/method.py index 27158cca..26408de6 100644 --- a/src/wakepy/core/method.py +++ b/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 diff --git a/src/wakepy/dbus_adapters/jeepney.py b/src/wakepy/dbus_adapters/jeepney.py index b5bf78e8..875d5476 100644 --- a/src/wakepy/dbus_adapters/jeepney.py +++ b/src/wakepy/dbus_adapters/jeepney.py @@ -64,3 +64,6 @@ def _create_new_connection( # type: ignore[no-any-unimported] ) from exc else: raise + + def close_connection(self, connection: DBusConnection): + connection.close() From 3f955c5bd7369dcf5bc3a72759c0ac86319c0985 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Niko=20F=C3=B6hr?= Date: Thu, 16 May 2024 22:07:42 +0300 Subject: [PATCH 3/7] fix mypy issues --- src/wakepy/core/dbus.py | 4 ++-- src/wakepy/dbus_adapters/jeepney.py | 8 +++++--- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/src/wakepy/core/dbus.py b/src/wakepy/core/dbus.py index 7f59dc5e..6c99b0fe 100644 --- a/src/wakepy/core/dbus.py +++ b/src/wakepy/core/dbus.py @@ -318,7 +318,7 @@ def _create_new_connection( """ raise NotImplementedError("Implement in subclass") - def close_connections(self): + def close_connections(self) -> None: """Close all the connections open in this adapter.""" for bus in list(self._connections): @@ -334,7 +334,7 @@ def close_connections(self): # See: https://github.com/fohrloop/wakepy/issues/277 gc.collect() - def close_connection(self, connection: object): + def close_connection(self, connection: object) -> None: """Close a dbus connection. Implement in a subclass""" raise NotImplementedError("Implement in subclass") diff --git a/src/wakepy/dbus_adapters/jeepney.py b/src/wakepy/dbus_adapters/jeepney.py index 875d5476..26a26622 100644 --- a/src/wakepy/dbus_adapters/jeepney.py +++ b/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: @@ -65,5 +67,5 @@ def _create_new_connection( # type: ignore[no-any-unimported] else: raise - def close_connection(self, connection: DBusConnection): + def close_connection(self, connection: DBusConnection) -> None: connection.close() From ed38656378ddcc6d1d5c15865bb03a6721da57c5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Niko=20F=C3=B6hr?= Date: Sat, 18 May 2024 21:03:14 +0300 Subject: [PATCH 4/7] remove the warning filter introduced in PR278 See: https://github.com/fohrloop/wakepy/pull/278/ --- tests/integration/conftest.py | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/tests/integration/conftest.py b/tests/integration/conftest.py index 083b9d0b..8447f9a0 100644 --- a/tests/integration/conftest.py +++ b/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") From ea8d09c38872dc49b7bf62debc18aa8013d13b82 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Niko=20F=C3=B6hr?= Date: Sat, 18 May 2024 21:21:58 +0300 Subject: [PATCH 5/7] refactor jeepney dbus adapter tests Use classes instead of functions --- tests/integration/test_dbus_adapters.py | 197 ++++++++++++------------ 1 file changed, 96 insertions(+), 101 deletions(-) diff --git a/tests/integration/test_dbus_adapters.py b/tests/integration/test_dbus_adapters.py index b478f2aa..46da8665 100644 --- a/tests/integration/test_dbus_adapters.py +++ b/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,10 @@ 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_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 From d1d3ae32dca126d94343fa709fb848bc96d8ee8e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Niko=20F=C3=B6hr?= Date: Sat, 18 May 2024 21:33:56 +0300 Subject: [PATCH 6/7] add test for .close_connections() --- tests/integration/test_dbus_adapters.py | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/tests/integration/test_dbus_adapters.py b/tests/integration/test_dbus_adapters.py index 46da8665..339333bd 100644 --- a/tests/integration/test_dbus_adapters.py +++ b/tests/integration/test_dbus_adapters.py @@ -177,6 +177,16 @@ def test_random_keyerror_on_connection_creation( 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) From 2009dff51ffeba994c84e7466e1fd835252f2e42 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Niko=20F=C3=B6hr?= Date: Sat, 18 May 2024 21:43:14 +0300 Subject: [PATCH 7/7] add warning note on org.gnome.SessionManager docs --- docs/source/methods-reference.md | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/docs/source/methods-reference.md b/docs/source/methods-reference.md index 3a806eb7..f5c5021c 100644 --- a/docs/source/methods-reference.md +++ b/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`