Skip to content

Commit

Permalink
RHEL-15110: RegisterServer is stopped, when not needed
Browse files Browse the repository at this point in the history
* When some application starts RegisterServer using Start(),
  then it returns address. When this method is called multiple
  times and RegisterServer is still running, then the same
  address is returned. It means, when user starts multiple
  applications, then all of these applications should be able
  to use the same address to keep backward compatibility.
  The RegisterServer should be terminated only in the case,
  when the last application call Stop() and there is no running
  proccess using this RegisterServer.
* This change does not allow to stop RegisterServer by some other
  user or application using Stop() method, when RegisterServer
  is still needed.
* Non-root user can stop the RegisterServer only in the situation,
  when it is not needed by any application.
* Implementation of Stop() method did not return anything explicitly,
  but the D-Bus method was designed to return boolean value. Thus,
  None object was interpreted as "False" value. It was fixed and it
  returns "True", when it was really stoped and it return "False",
  when it was not possible to stop it, because some application
  still uses RegisterServer.
* It looks like that there was probably intention to use similar
  approach, but it has never been finished.
* Modified some unit tests
  • Loading branch information
jirihnidek authored and ptoscano committed Jan 5, 2024
1 parent ba1e0e4 commit bc33776
Show file tree
Hide file tree
Showing 4 changed files with 84 additions and 7 deletions.
7 changes: 7 additions & 0 deletions src/rhsmlib/dbus/dbus_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,13 @@ def pid_of_sender(bus, sender):
pid = int(dbus_iface.GetConnectionUnixProcessID(sender))
except ValueError:
return None
# It seems that Python D-Bus implementation contains error. This should not happen,
# when we try to call GetConnectionUnixProcessID() with sender that does not exist
# anymore
except dbus.exceptions.DBusException as err:
log.debug(f"D-Bus raised exception: {err}")
return None

return pid


Expand Down
21 changes: 18 additions & 3 deletions src/rhsmlib/dbus/objects/register.py
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,13 @@ def start(self, sender: str) -> str:
:return: Server address.
"""
with self.lock:
# When some other application already started domain socket listener, then
# write log message and return existing address
if self.server is not None:
log.debug(f"Domain socket listener already running, started by: {self.server.sender}")
# Add sender to the list of senders using server
log.debug(f"Adding another sender {sender} to the set of senders")
self.server.add_sender(sender)
return self.server.address

log.debug("Trying to create new domain socket server.")
Expand All @@ -66,7 +72,7 @@ def start(self, sender: str) -> str:
)
return address

def stop(self) -> None:
def stop(self, sender: str) -> bool:
"""Stop the server running on the domain socket.
:raises exceptions.Failed: No domain socket server is running.
Expand All @@ -75,9 +81,18 @@ def stop(self) -> None:
if self.server is None:
raise exceptions.Failed("No domain socket server is running")

# Remove current sender and check if other senders are still running.
# If there is at least one sender using this server still running, then
# only return False
self.server.remove_sender(sender)
if self.server.are_other_senders_still_running() is True:
log.debug("Not stopping domain socket server, because some senders still uses it.")
return False

self.server.shutdown()
self.server = None
log.debug("Domain socket server stopped.")
return True


class RegisterDBusObject(base_object.BaseObject):
Expand Down Expand Up @@ -109,11 +124,11 @@ def Start(self, locale, sender=None):
)
@util.dbus_handle_sender
@util.dbus_handle_exceptions
def Stop(self, locale, sender=None):
def Stop(self, locale, sender=None) -> bool:
locale = dbus_utils.dbus_to_python(locale, expected_type=str)
Locale.set(locale)

self.impl.stop()
return self.impl.stop(sender)


class OrgNotSpecifiedException(dbus.DBusException):
Expand Down
43 changes: 42 additions & 1 deletion src/rhsmlib/dbus/server.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
from functools import partial

from rhsmlib.services import config
from rhsmlib.dbus.dbus_utils import pid_of_sender
from rhsm.config import get_config_parser
from rhsmlib.file_monitor import create_filesystem_watcher, DirectoryWatch
from rhsmlib.file_monitor import (
Expand Down Expand Up @@ -323,13 +324,53 @@ def __init__(self, object_classes=None, sender=None, cmd_line=None):
self.object_classes = object_classes or []
self.objects = []
self._server = None
self.sender = sender
self.sender = sender # sender created the server
self._senders = set() # other senders using server
log.debug(f"Adding sender {sender} to the set of senders")
self._senders.add(sender)
self.cmd_line = cmd_line

self.lock = threading.Lock()
with self.lock:
self.connection_count = 0

def add_sender(self, sender: str) -> None:
"""
Add sender to the list of senders
"""
self._senders.add(sender)

def remove_sender(self, sender: str) -> bool:
"""
Try to remove sender from the set of sender
"""
try:
self._senders.remove(sender)
except KeyError:
log.debug(f"Sender {sender} wasn't removed from the set of senders (not member of the set)")
return False
else:
log.debug(f"Sender {sender} removed from the set of senders")
return True

def are_other_senders_still_running(self) -> bool:
"""
Check if other users are still running. It sender in the set is not
running, then remove sender from the set, because sender could
crash, or it was terminated since it called Start() method.
"""
is_one_sender_running = False
not_running = set()
bus = dbus.SystemBus()
for sender in self._senders:
pid = pid_of_sender(bus, sender)
if pid is None:
not_running.add(sender)
else:
is_one_sender_running = True
self._senders = self._senders.difference(not_running)
return is_one_sender_running

def shutdown(self):
for o in self.objects:
o.remove_from_connection()
Expand Down
20 changes: 17 additions & 3 deletions test/rhsmlib/dbus/test_register.py
Original file line number Diff line number Diff line change
Expand Up @@ -340,12 +340,26 @@ def setUp(self) -> None:
get_cmd_line_mock = get_cmd_line_patch.start()
get_cmd_line_mock.return_value = "unit-test sender"

pid_of_sender_patch = mock.patch(
"rhsmlib.dbus.server.pid_of_sender",
autospec=True,
)
pid_of_sender_mock = pid_of_sender_patch.start()
pid_of_sender_mock.return_value = 123456

are_others_running_path = mock.patch(
"rhsmlib.dbus.server.DomainSocketServer.are_other_senders_still_running",
autospec=True,
)
are_others_running_mock = are_others_running_path.start()
are_others_running_mock.return_value = False

dbus.mainloop.glib.DBusGMainLoop(set_as_default=True)

def tearDown(self) -> None:
"""Make sure the domain server is stopped once the test ends."""
with contextlib.suppress(rhsmlib.dbus.exceptions.Failed):
self.impl.stop()
self.impl.stop("sender")

super().tearDown()

Expand Down Expand Up @@ -373,11 +387,11 @@ def test_Start__can_connect(self):

def test_Stop(self):
self.impl.start("sender")
self.impl.stop()
self.impl.stop("sender")

def test_Stop__not_running(self):
with self.assertRaises(rhsmlib.dbus.exceptions.Failed):
self.impl.stop()
self.impl.stop("sender")


class DomainSocketRegisterDBusObjectTest(SubManDBusFixture):
Expand Down

0 comments on commit bc33776

Please sign in to comment.