diff --git a/.github/workflows/coverage.yml b/.github/workflows/coverage.yml index bf76ad3e7b6..224efbaca67 100644 --- a/.github/workflows/coverage.yml +++ b/.github/workflows/coverage.yml @@ -22,6 +22,7 @@ jobs: requirements: requirements-test.txt - name: Run Pytest with Coverage + timeout-minutes: 7 run: | coverage run --source=./src/tribler/core -p -m pytest ./src/tribler/core --looptime coverage run --source=./src/tribler/core -p -m pytest ./src/tribler/core/components/tunnel/tests/test_full_session --tunneltests --looptime diff --git a/src/run_tribler_headless.py b/src/run_tribler_headless.py index 74cd7e5ba17..1785ddba528 100644 --- a/src/run_tribler_headless.py +++ b/src/run_tribler_headless.py @@ -63,7 +63,7 @@ async def signal_handler(sig): await self.session.shutdown() print("Tribler shut down") get_event_loop().stop() - self.process_checker.remove_lock_file() + self.process_checker.remove_lock() signal.signal(signal.SIGINT, lambda sig, _: ensure_future(signal_handler(sig))) signal.signal(signal.SIGTERM, lambda sig, _: ensure_future(signal_handler(sig))) @@ -73,11 +73,10 @@ async def signal_handler(sig): # Check if we are already running a Tribler instance root_state_dir = get_root_state_directory() + self.process_checker = ProcessChecker(root_state_dir) - if self.process_checker.already_running: - print(f"Another Tribler instance is already using statedir {config.state_dir}") - get_event_loop().stop() - return + self.process_checker.check_and_restart_if_necessary() + self.process_checker.create_lock() print("Starting Tribler") diff --git a/src/tribler/core/check_os.py b/src/tribler/core/check_os.py index 8827d463ea3..832273306c8 100644 --- a/src/tribler/core/check_os.py +++ b/src/tribler/core/check_os.py @@ -7,12 +7,8 @@ import psutil -from tribler.core.utilities.process_checker import ProcessChecker from tribler.core.utilities.utilities import show_system_popup -FORCE_RESTART_MESSAGE = "An existing Tribler core process (PID:%s) is already running. \n\n" \ - "Do you want to stop the process and do a clean restart instead?" - logger = logging.getLogger(__name__) @@ -62,124 +58,6 @@ def check_free_space(): error_and_exit("Import Error", f"Import error: {ie}") -def get_existing_tribler_pid(root_state_dir): - """ Get PID of existing instance if present from the lock file (if any)""" - process_checker = ProcessChecker(root_state_dir) - if process_checker.already_running: - return process_checker.get_pid_from_lock_file() - return -1 - - -def should_kill_other_tribler_instances(root_state_dir): - """ Asks user whether to force restart Tribler if there is more than one instance running. - This will help user to kill any zombie instances which might have been left behind from - previous force kill command or some other unexpected exceptions and relaunch Tribler again. - It ignores if Tribler is opened with some arguments, for eg. with a torrent. - """ - logger.info('Should kill other Tribler instances') - - # If there are cmd line args, let existing instance handle it - if len(sys.argv) > 1: - return - - old_pid = get_existing_tribler_pid(root_state_dir) - current_pid = os.getpid() - logger.info(f'Old PID: {old_pid}. Current PID: {current_pid}') - - if current_pid != old_pid and old_pid > 0: - # If the old process is a zombie, simply kill it and restart Tribler - old_process = psutil.Process(old_pid) - try: - old_process_status = old_process.status() - except psutil.NoSuchProcess: - logger.info('Old process not found') - return - - logger.info(f'Old process status: {old_process_status}') - if old_process_status == psutil.STATUS_ZOMBIE: - kill_tribler_process(old_process) - restart_tribler_properly() - return - - from PyQt5.QtWidgets import QApplication, QMessageBox - app = QApplication(sys.argv) # pylint: disable=W0612 - message_box = QMessageBox() - message_box.setWindowTitle("Warning") - message_box.setText("Warning") - message_box.setInformativeText(FORCE_RESTART_MESSAGE % old_pid) - message_box.setStandardButtons(QMessageBox.Yes | QMessageBox.No) - message_box.setDefaultButton(QMessageBox.Save) - result = message_box.exec_() - - if result == QMessageBox.Yes: - kill_tribler_process(old_process) - restart_tribler_properly() - else: - sys.exit(0) - - -def is_tribler_process(name): - """ - Checks if the given name is of a Tribler processs. It checks a few potential keywords that - could be present in a Tribler process name across different platforms. - :param name: Process name - :return: True if pid is a Tribler process else False - """ - name = name.lower() - keywords = ['tribler', 'python'] - - result = any(keyword in name for keyword in keywords) - logger.info(f'Is Tribler process: {result}') - return result - - -def kill_tribler_process(process): - """ - Kills the given process if it is a Tribler process. - :param process: psutil.Process - :return: None - """ - logger.info(f'Kill Tribler process: {process}') - - try: - if not is_tribler_process(process.exe()): - return - - parent_process = process.parent() - logger.info(f'Parent process: {parent_process}') - - if parent_process.pid > 1 and is_tribler_process(parent_process.exe()): - logger.info(f'OS kill: {process.pid} and {parent_process.pid}') - os.kill(process.pid, 9) - os.kill(parent_process.pid, 9) - else: - logger.info(f'OS kill: {process.pid} ') - os.kill(process.pid, 9) - - except OSError: - logger.exception("Failed to kill the existing Tribler process") - - -def restart_tribler_properly(): - """ - Restarting Tribler with proper cleanup of file objects and descriptors - """ - logger.info('Restart Tribler properly') - try: - process = psutil.Process(os.getpid()) - for handler in process.open_files() + process.connections(): - logger.info(f'OS close: {handler}') - os.close(handler.fd) - except Exception as e: - # If exception occurs on cleaning up the resources, simply log it and continue with the restart - logger.error(e) - - python = sys.executable - - logger.info(f'OS execl: "{python}". Args: "{sys.argv}"') - os.execl(python, python, *sys.argv) - - def set_process_priority(pid=None, priority_order=1): """ Sets process priority based on order provided. Note order range is 0-5 and higher value indicates higher priority. diff --git a/src/tribler/core/start_core.py b/src/tribler/core/start_core.py index 6a95e624e4b..8c333318973 100644 --- a/src/tribler/core/start_core.py +++ b/src/tribler/core/start_core.py @@ -10,7 +10,6 @@ from tribler.core.check_os import ( check_and_enable_code_tracing, set_process_priority, - should_kill_other_tribler_instances, ) from tribler.core.components.bandwidth_accounting.bandwidth_accounting_component import BandwidthAccountingComponent from tribler.core.components.base import Component, Session @@ -36,7 +35,7 @@ from tribler.core.logger.logger import load_logger_config from tribler.core.sentry_reporter.sentry_reporter import SentryReporter, SentryStrategy from tribler.core.upgrade.version_manager import VersionHistory -from tribler.core.utilities.process_checker import ProcessChecker +from tribler.core.utilities.process_checker import single_tribler_instance logger = logging.getLogger(__name__) CONFIG_FILE_NAME = 'triblerd.conf' @@ -162,20 +161,10 @@ def run_tribler_core_session(api_port, api_key, state_dir, gui_test_mode=False): def run_core(api_port, api_key, root_state_dir, parsed_args): - should_kill_other_tribler_instances(root_state_dir) logger.info('Running Core' + ' in gui_test_mode' if parsed_args.gui_test_mode else '') load_logger_config('tribler-core', root_state_dir) - # Check if we are already running a Tribler instance - process_checker = ProcessChecker(root_state_dir) - if process_checker.already_running: - logger.info('Core is already running, exiting') - sys.exit(1) - process_checker.create_lock_file() - version_history = VersionHistory(root_state_dir) - state_dir = version_history.code_version.directory - try: + with single_tribler_instance(root_state_dir): + version_history = VersionHistory(root_state_dir) + state_dir = version_history.code_version.directory run_tribler_core_session(api_port, api_key, state_dir, gui_test_mode=parsed_args.gui_test_mode) - finally: - logger.info('Remove lock file') - process_checker.remove_lock_file() diff --git a/src/tribler/core/tests/test_check_os.py b/src/tribler/core/tests/test_check_os.py index 9322d21a9d3..9f4f2eae039 100644 --- a/src/tribler/core/tests/test_check_os.py +++ b/src/tribler/core/tests/test_check_os.py @@ -1,18 +1,14 @@ from logging import Logger from unittest.mock import MagicMock, patch -import psutil - -import pytest - -from tribler.core.check_os import enable_fault_handler, error_and_exit, should_kill_other_tribler_instances +from tribler.core.check_os import enable_fault_handler, error_and_exit from tribler.core.utilities.patch_import import patch_import + # pylint: disable=import-outside-toplevel # fmt: off - @patch('sys.exit') @patch('tribler.core.check_os.show_system_popup') async def test_error_and_exit(mocked_show_system_popup, mocked_sys_exit): @@ -45,35 +41,3 @@ async def test_enable_fault_handler_log_dir_not_exists(): enable_fault_handler(log_dir=log_dir) log_dir.mkdir.assert_called_once() - - -@patch('tribler.core.check_os.logger.info') -@patch('sys.argv', []) -@patch('tribler.core.check_os.get_existing_tribler_pid', MagicMock(return_value=100)) -@patch('os.getpid', MagicMock(return_value=200)) -@patch('psutil.Process', MagicMock(return_value=MagicMock(status=MagicMock(side_effect=psutil.NoSuchProcess(100))))) -def test_should_kill_other_tribler_instances_process_not_found( - mocked_logger_info: MagicMock -): - root_state_dir = MagicMock() - should_kill_other_tribler_instances(root_state_dir) - mocked_logger_info.assert_called_with('Old process not found') - - -@patch('tribler.core.check_os.logger.info') -@patch('sys.argv', []) -@patch('tribler.core.check_os.get_existing_tribler_pid', MagicMock(return_value=100)) -@patch('os.getpid', MagicMock(return_value=200)) -@patch('psutil.Process', MagicMock(return_value=MagicMock(status=MagicMock(return_value=psutil.STATUS_ZOMBIE)))) -@patch('tribler.core.check_os.kill_tribler_process') -@patch('tribler.core.check_os.restart_tribler_properly') -def test_should_kill_other_tribler_instances_zombie( - mocked_restart_tribler_properly: MagicMock, - mocked_kill_tribler_process: MagicMock, - mocked_logger_info: MagicMock, -): - root_state_dir = MagicMock() - should_kill_other_tribler_instances(root_state_dir) - mocked_logger_info.assert_called() - mocked_kill_tribler_process.assert_called_once() - mocked_restart_tribler_properly.assert_called_once() diff --git a/src/tribler/core/utilities/process_checker.py b/src/tribler/core/utilities/process_checker.py index f38ae68d04d..429230bb19e 100644 --- a/src/tribler/core/utilities/process_checker.py +++ b/src/tribler/core/utilities/process_checker.py @@ -1,9 +1,26 @@ +from __future__ import annotations + +import logging import os -from pathlib import Path +import re +import sys +from contextlib import contextmanager +from typing import Iterable, Optional import psutil -LOCK_FILE_NAME = 'triblerd.lock' +from tribler.core.utilities.path_util import Path + + +@contextmanager +def single_tribler_instance(directory: Path): + checker = ProcessChecker(directory) + try: + checker.check_and_restart_if_necessary() + checker.create_lock() + yield checker + finally: + checker.remove_lock() class ProcessChecker: @@ -11,63 +28,138 @@ class ProcessChecker: This class contains code to check whether a Tribler process is already running. """ - def __init__(self, state_directory: Path): - self.state_directory = state_directory - self.lock_file_path = self.state_directory / LOCK_FILE_NAME + def __init__(self, directory: Path, lock_file_name: str = 'tribler.lock'): + self.logger = logging.getLogger(self.__class__.__name__) + self.lock_file = directory / lock_file_name + self.re_tribler = re.compile(r'tribler\b(?![/\\])') + self.logger.info(f'Lock file: {self.lock_file}') - if self.lock_file_path.exists(): - # Check for stale lock file (created before the os was last restarted). - # The stale file might contain the pid of another running process and - # not the Tribler itself. To find out we can simply check if the lock file - # was last modified before os reboot. - # lock_file_modification_time < system boot time - file_pid = self.get_pid_from_lock_file() - if file_pid < 1 or self.lock_file_path.stat().st_mtime < psutil.boot_time(): - self.remove_lock_file() + def check_and_restart_if_necessary(self) -> bool: + self.logger.info('Check') - self.already_running = self.is_process_running() + pid = self._get_pid_from_lock() + try: + process = psutil.Process(pid) + status = process.status() + except psutil.Error as e: + self.logger.warning(e) + return False - def is_process_running(self): - if self.lock_file_path.exists(): - file_pid = self.get_pid_from_lock_file() + if not self._is_old_tribler_process_running(process): + return False - if file_pid == os.getpid() or ProcessChecker.is_pid_running(file_pid): - return True - return False + if status == psutil.STATUS_ZOMBIE: + self._close_process(process) + self._restart_tribler() + return True - @staticmethod - def is_pid_running(pid): - return psutil.pid_exists(pid) + self._ask_to_restart(process) + return True - def create_lock_file(self): - """ - Create the lock file and write the PID in it. We also create the directory structure since the ProcessChecker - might be called before the .Tribler directory has been created. - """ - if not self.state_directory.exists(): - Path(self.state_directory).mkdir(parents=True) + def create_lock(self, pid: Optional[int] = None): + self.logger.info('Create the lock file') - # Remove the previous lock file - self.remove_lock_file() + pid = pid or os.getpid() + try: + self.lock_file.parent.mkdir(exist_ok=True) + self.lock_file.write_text(f'{pid}') + except Exception as e: # pylint: disable=broad-except + self.logger.exception(e) - with self.lock_file_path.open(mode='wb') as lock_file: - lock_file.write(str(os.getpid()).encode()) + def remove_lock(self): + self.logger.info('Remove the lock file') - def remove_lock_file(self): - """ - Remove the lock file if it exists. - """ - if self.lock_file_path.exists(): - self.lock_file_path.unlink(missing_ok=True) + try: + self.lock_file.unlink(missing_ok=True) + except Exception as e: # pylint: disable=broad-except + self.logger.exception(e) - def get_pid_from_lock_file(self): + def _get_pid_from_lock(self) -> Optional[int]: """ Returns the PID from the lock file. """ - if not self.lock_file_path.exists(): - return -1 - with self.lock_file_path.open(mode='rb') as lock_file: - try: - return int(lock_file.read()) - except ValueError: - return -1 + self.logger.info('Get PID from the lock file') + try: + pid = int(self.lock_file.read_text()) + self.logger.info(f'PID is {pid}') + return pid + except Exception as e: # pylint: disable=broad-except + self.logger.warning(e) + + return None + + def _is_tribler_cmd(self, cmd_line: Optional[Iterable[str]]) -> bool: + cmd_line = cmd_line or [] + cmd = ''.join(cmd_line).lower() + self.logger.info(f'Check process cmd: {cmd}') + + return self.re_tribler.search(cmd) is not None + + def _is_old_tribler_process_running(self, process: psutil.Process) -> bool: + cmdline = process.as_dict()['cmdline'] + + has_keyword = self._is_tribler_cmd(cmdline) + pid_is_exists = psutil.pid_exists(process.pid) + pid_is_correct = process.pid > 1 and process.pid != os.getpid() + + result = has_keyword and pid_is_exists and pid_is_correct + self.logger.info(f'Result: {result} (has_keyword={has_keyword}, ' + f'pid_is_exists={pid_is_exists}, pid_is_correct={pid_is_correct})') + + return result + + def _ask_to_restart(self, process: psutil.Process): + self.logger.info('Ask to restart') + + try: + self._close_process(process) + + from PyQt5.QtWidgets import QApplication, QMessageBox # pylint: disable=import-outside-toplevel + _ = QApplication(sys.argv) + message_box = QMessageBox() + message_box.setWindowTitle("Warning") + message_box.setText("Warning") + message_box.setInformativeText( + f"An existing Tribler core process (PID:{process.pid}) is already running. \n\n" + f"Do you want to stop the process and do a clean restart instead?" + ) + message_box.setStandardButtons(QMessageBox.Yes | QMessageBox.No) + message_box.setDefaultButton(QMessageBox.Save) + result = message_box.exec_() + if result == QMessageBox.Yes: + self.logger.info('Ask to restart (yes)') + self._restart_tribler() + except Exception as e: # pylint: disable=broad-except + self.logger.exception(e) + + def _close_process(self, process: psutil.Process): + def close_handlers(): + for handler in process.open_files() + process.connections(): + self.logger.info(f'OS close: {handler}') + try: + os.close(handler.fd) + except Exception as e: # pylint: disable=broad-except + self.logger.warning(e) + + def kill_processes(): + processes_to_kill = [process, process.parent()] + self.logger.info(f'Kill Tribler processes: {processes_to_kill}') + for p in processes_to_kill: + try: + if self._is_old_tribler_process_running(p): + self.logger.info(f'Kill: {p.pid}') + os.kill(p.pid, 9) + except OSError as e: + self.logger.exception(e) + + close_handlers() + kill_processes() + + def _restart_tribler(self): + """ Restart Tribler + """ + self.logger.info('Restart Tribler') + + python = sys.executable + self.logger.info(f'OS execl: "{python}". Args: "{sys.argv}"') + os.execl(python, python, *sys.argv) # See: https://github.com/Tribler/tribler/issues/6948 diff --git a/src/tribler/core/utilities/tests/test_osutils.py b/src/tribler/core/utilities/tests/test_osutils.py index 51c265820fb..514e492fc28 100644 --- a/src/tribler/core/utilities/tests/test_osutils.py +++ b/src/tribler/core/utilities/tests/test_osutils.py @@ -12,8 +12,6 @@ is_android, ) -from tribler.core.check_os import is_tribler_process - def test_fix_filebasename(): default_name = '_' @@ -126,15 +124,3 @@ def test_dir_copy(tmpdir): dir_copy(src_dir, dest_dir2, merge_if_exists=True) assert len(os.listdir(src_dir)) == len(os.listdir(dest_dir2)) assert Path(dest_dir2, dummy_file).read_text() == "source: hello world" - - -def test_is_tribler_process(): - assert is_tribler_process('python.exe') - assert is_tribler_process('run_tribler.py') - assert is_tribler_process('usr/bin/python') - assert is_tribler_process('usr/bin/tribler') - assert is_tribler_process('Tribler.exe') - assert is_tribler_process('Tribler.sh') - assert is_tribler_process('Contents/MacOS/tribler') - - assert not is_tribler_process('any other string') diff --git a/src/tribler/core/utilities/tests/test_process_checker.py b/src/tribler/core/utilities/tests/test_process_checker.py index 462bea61786..b1b9f23e79d 100644 --- a/src/tribler/core/utilities/tests/test_process_checker.py +++ b/src/tribler/core/utilities/tests/test_process_checker.py @@ -1,94 +1,268 @@ -import os from multiprocessing import Process -from pathlib import Path from time import sleep +from unittest.mock import MagicMock, Mock, patch +import psutil import pytest +from PyQt5.QtWidgets import QMessageBox -from tribler.core.utilities.process_checker import LOCK_FILE_NAME, ProcessChecker +from tribler.core.utilities.patch_import import patch_import +from tribler.core.utilities.path_util import Path +from tribler.core.utilities.process_checker import ProcessChecker, single_tribler_instance +TRIBLER_CMD_LINE = [ + ['usr/bin/python', 'run_tribler.py'], + [r'c:\Program Files\Tribler\Tribler.exe'], + [r'c:\Program Files\Tribler\Tribler.exe', 'some.torrent'], + ['Tribler.sh'], + ['Contents/MacOS/tribler'], +] + +NOT_TRIBLER_CMD_LINE = [ + None, + ['usr/bin/python'], + [r'c:\Program Files\Tribler\any.exe'], + [r'tribler\any\path'], +] + + +# pylint: disable=redefined-outer-name, protected-access @pytest.fixture -def process_checker(tmpdir): - return ProcessChecker(state_directory=Path(tmpdir)) +def checker(tmp_path): + return ProcessChecker(directory=Path(tmp_path)) -def process_dummy_function(): - while True: - sleep(0.01) +def idle(): + sleep(100) @pytest.fixture -def background_process(): - process = Process(target=process_dummy_function) +def process(): + process = Process(target=idle) process.start() - yield process + yield psutil.Process(process.pid) process.terminate() -def create_lock_file_with_pid(tmpdir, pid): - with open(tmpdir / LOCK_FILE_NAME, 'w') as lock_file: - lock_file.write(str(pid)) +def test_get_pid_lock_file(checker: ProcessChecker): + # Test that previously saved PID can be read. + checker.lock_file.write_text('42') + assert checker._get_pid_from_lock() == 42 + + +def test_get_wrong_pid_lock_file(checker: ProcessChecker): + # Test that in the case of inconsistent PID None will be returned. + checker.lock_file.write_text('string') + assert checker._get_pid_from_lock() is None + + +@patch.object(Path, 'read_text', Mock(side_effect=PermissionError)) +def test_permission_denied(checker: ProcessChecker): + # Test that in the case of any Exception, None will be returned. + checker.lock_file.write_text('42') + + assert checker._get_pid_from_lock() is None + + +def test_missed_lock_file(checker: ProcessChecker): + # Test that in the case of a missed lock file, None will be returned. + assert checker._get_pid_from_lock() is None + + +@patch.object(psutil.Process, 'as_dict', Mock(return_value={'cmdline': None})) +def test_is_old_tribler_process_cmdline_none(checker: ProcessChecker, process: psutil.Process): + # Test that in the case of a missed `cmdline`, False will be returned. + assert not checker._is_old_tribler_process_running(process) + + +@patch.object(psutil.Process, 'as_dict', Mock(return_value={'cmdline': r'some\path\tribler'})) +def test_is_old_tribler_process(checker: ProcessChecker, process: psutil.Process): + # Test that in the case keyword 'tribler' is somewhere in `cmdline', True will be returned. + assert checker._is_old_tribler_process_running(process) + + +@patch.object(psutil.Process, 'as_dict', Mock(return_value={'cmdline': r'some\path'})) +def test_is_not_old_tribler_process(checker: ProcessChecker, process: psutil.Process): + # Test that in the case keyword 'tribler' is not somewhere in `cmdline', False will be returned. + assert not checker._is_old_tribler_process_running(process) + + +def test_create_lock(checker: ProcessChecker): + # Test that the lock file can be created and read. + assert not checker._get_pid_from_lock() + + checker.create_lock() + + assert isinstance(checker._get_pid_from_lock(), int) + + +def test_create_lock_sub_folder(tmp_path): + # Test that the lock file can be created in a folder that does not exist. + checker = ProcessChecker(directory=tmp_path / 'sub folder') + checker.create_lock() + + assert checker._get_pid_from_lock() + + +@patch.object(Path, 'write_text', Mock(side_effect=PermissionError)) +def test_create_lock_exception(checker: ProcessChecker): + # Test that the lock file can not be created in the case of any Exception. + checker.create_lock() + + assert not checker._get_pid_from_lock() + + +def test_remove_lock(checker: ProcessChecker): + # Test that the lock file can be removed. + checker.create_lock() + assert checker._get_pid_from_lock() + + checker.remove_lock() + assert not checker._get_pid_from_lock() + + +@patch.object(Path, 'unlink', Mock(side_effect=PermissionError)) +def test_remove_lock_with_errors(checker: ProcessChecker): + # Test that the lock file can not be removed in the case of any exception. + checker.create_lock() + checker.remove_lock() + + assert checker._get_pid_from_lock() + + +@patch.object(ProcessChecker, 'check_and_restart_if_necessary', Mock()) +@patch.object(ProcessChecker, 'create_lock', Mock()) +@patch.object(ProcessChecker, 'remove_lock', Mock()) +def test_contextmanager(tmp_path): + # Test that all necessary methods have been called during the context manager using. + with single_tribler_instance(tmp_path) as checker: + assert checker.check_and_restart_if_necessary.called + assert checker.create_lock.called + assert not checker.remove_lock.called + + assert checker.remove_lock.called + + +@patch.object(ProcessChecker, '_restart_tribler', Mock()) +@patch.object(ProcessChecker, '_ask_to_restart', Mock()) +@patch.object(psutil.Process, 'as_dict', Mock(return_value={'cmdline': r'tribler'})) +def test_check(checker: ProcessChecker, process: psutil.Process): + # Ensure that `_restart_tribler` and `_ask_to_restart` methods have been called when + # tribler process with a proper PID has been checked. + # Here process is a fake process. + assert not checker.check_and_restart_if_necessary() + assert not checker._restart_tribler.called + assert not checker._ask_to_restart.called + + checker.create_lock(process.pid) + + assert checker.check_and_restart_if_necessary() + assert checker._ask_to_restart.called + assert not checker._restart_tribler.called + + +@patch.object(ProcessChecker, '_restart_tribler', Mock()) +@patch.object(ProcessChecker, '_ask_to_restart', Mock()) +@patch.object(psutil.Process, 'as_dict', Mock(return_value={'cmdline': r'tribler'})) +@patch.object(psutil.Process, 'status', Mock(return_value=psutil.STATUS_ZOMBIE)) +def test_check_zombie(checker: ProcessChecker, process: psutil.Process): + # Ensure that the `_restart_tribler` method has been called when + # tribler process with a proper PID has been checked. + # Here process is a fake process. + assert not checker.check_and_restart_if_necessary() + assert not checker._restart_tribler.called + assert not checker._ask_to_restart.called + + checker.create_lock(process.pid) + + assert checker.check_and_restart_if_necessary() + assert not checker._ask_to_restart.called + assert checker._restart_tribler.called + + +@patch.object(psutil.Process, 'status', Mock(side_effect=psutil.Error)) +def test_check_psutil_error(checker: ProcessChecker): + # Ensure that the `check` method don`t raise an exception in the case `psutil.Process.status()` + # raises `psutil.Error` exception. + assert not checker.check_and_restart_if_necessary() + + +@pytest.mark.parametrize('cmd_line', TRIBLER_CMD_LINE) +def test_is_tribler_cmd(cmd_line, checker: ProcessChecker): + assert checker._is_tribler_cmd(cmd_line) + + +@pytest.mark.parametrize('cmd_line', NOT_TRIBLER_CMD_LINE) +def test_not_is_tribler_cmd(cmd_line, checker: ProcessChecker): + assert not checker._is_tribler_cmd(cmd_line) -def test_create_lock_file(tmpdir, process_checker): - """ - Testing if lock file is created - """ - process_checker.create_lock_file() - assert (tmpdir / LOCK_FILE_NAME).exists() +@patch.object(ProcessChecker, '_restart_tribler', Mock()) +@patch.object(ProcessChecker, '_close_process', Mock()) +def test_ask_to_restart_yes(checker: ProcessChecker, process: psutil.Process): + # Ensure that when a user choose "Yes" in the message box from the `_ask_to_restart` method, + # `_restart_tribler` is called. + mocked_QApplication = Mock() + mocked_QMessageBox = MagicMock(Yes=QMessageBox.Yes, + return_value=MagicMock(exec_=Mock(return_value=QMessageBox.Yes))) + with patch_import('PyQt5.QtWidgets', strict=True, QApplication=mocked_QApplication, QMessageBox=mocked_QMessageBox): + checker._ask_to_restart(process) + assert mocked_QMessageBox.called + assert mocked_QApplication.called + assert checker._restart_tribler.called + assert checker._close_process.called -def test_remove_lock_file(tmpdir, process_checker): - """ - Testing if lock file is removed on calling remove_lock_file() - """ - process_checker.create_lock_file() - process_checker.remove_lock_file() - assert not (tmpdir / LOCK_FILE_NAME).exists() +@patch.object(ProcessChecker, '_restart_tribler', Mock()) +@patch.object(ProcessChecker, '_close_process', Mock()) +def test_ask_to_restart_no(checker: ProcessChecker, process: psutil.Process): + # Ensure that when a user choose "No" in the message box from the `_ask_to_restart` method, + # `_close_process` is called. + mocked_QApplication = Mock() + mocked_QMessageBox = MagicMock(No=QMessageBox.No, + return_value=MagicMock(exec_=Mock(return_value=QMessageBox.No))) + with patch_import('PyQt5.QtWidgets', strict=True, QApplication=mocked_QApplication, QMessageBox=mocked_QMessageBox): + checker._ask_to_restart(process) -def test_no_lock_file(tmpdir, process_checker): - """ - Testing whether the process checker returns false when there is no lock file - """ - # Process checker does not create a lock file itself now, Core manager will call to create it. - assert not (tmpdir / LOCK_FILE_NAME).exists() - assert not process_checker.already_running + assert mocked_QMessageBox.called + assert mocked_QApplication.called + assert checker._close_process.called + assert not checker._restart_tribler.called -def test_invalid_pid_in_lock_file(tmpdir): - """ - Testing pid should be -1 if the lock file is invalid - """ - with open(tmpdir / LOCK_FILE_NAME, 'wb') as lock_file: - lock_file.write(b"Hello world") +@patch.object(ProcessChecker, '_restart_tribler', Mock()) +@patch.object(ProcessChecker, '_close_process', Mock()) +def test_ask_to_restart_error(checker: ProcessChecker, process: psutil.Process): + # Ensure that in the case of an error in `_ask_to_restart` method, + # `_close_process` is called. + checker._restart_tribler = MagicMock() + checker._close_process = Mock() + with patch_import('PyQt5.QtWidgets', always_raise_exception_on_import=True): + checker._ask_to_restart(process) - process_checker = ProcessChecker(state_directory=Path(tmpdir)) - assert process_checker.get_pid_from_lock_file() == -1 + assert not checker._restart_tribler.called + assert checker._close_process.called -def test_own_pid_in_lock_file(tmpdir): - """ - Testing whether the process checker returns True when it finds its own pid in the lock file - """ - create_lock_file_with_pid(tmpdir, os.getpid()) - process_checker = ProcessChecker(state_directory=Path(tmpdir)) - assert process_checker.already_running +@patch.object(psutil.Process, 'as_dict', Mock(return_value={'cmdline': r'tribler'})) +@patch('os.kill') +def test_close_process(mocked_kill: Mock, checker: ProcessChecker, process: psutil.Process): + checker._close_process(process) + assert mocked_kill.called -def test_other_instance_running(tmpdir, background_process): - """Testing whether the process checker returns true when another process is running.""" - create_lock_file_with_pid(tmpdir, background_process.pid) - process_checker = ProcessChecker(state_directory=Path(tmpdir)) - assert process_checker.is_pid_running(background_process.pid) - assert process_checker.already_running +@patch.object(psutil.Process, 'as_dict', Mock(return_value={'cmdline': r'tribler'})) +@patch('os.kill', Mock(side_effect=OSError)) +@patch('os.close', Mock(side_effect=OSError)) +def test_close_process_errors(checker: ProcessChecker, process: psutil.Process): + # Ensure that in the case `os.kill` or `os.close` raises an exception, the `_close_process` + # will never throw it further. + checker._close_process(process) -def test_dead_pid_in_lock_file(tmpdir): - """Testing whether the process checker returns false when there is a dead pid in the lock file.""" - dead_pid = 134824733 - create_lock_file_with_pid(tmpdir, dead_pid) - process_checker = ProcessChecker(state_directory=Path(tmpdir)) - assert not process_checker.is_pid_running(dead_pid) - assert not process_checker.already_running +@patch('os.execl') +def test_restart_tribler(mocked_execl: Mock, checker: ProcessChecker): + checker._restart_tribler() + assert mocked_execl.called diff --git a/src/tribler/core/utilities/tiny_tribler_service.py b/src/tribler/core/utilities/tiny_tribler_service.py index 7fb8fd1539f..dd1893d3621 100644 --- a/src/tribler/core/utilities/tiny_tribler_service.py +++ b/src/tribler/core/utilities/tiny_tribler_service.py @@ -2,7 +2,7 @@ import logging import signal from pathlib import Path -from typing import List +from typing import List, Optional from tribler.core.components.base import Component from tribler.core.start_core import Session @@ -20,7 +20,7 @@ def __init__(self, config, components: List[Component], timeout_in_sec=None, wor self.logger = logging.getLogger(self.__class__.__name__) self.session = None - self.process_checker = None + self.process_checker: Optional[ProcessChecker] = None self.working_dir = working_dir self.config = config self.timeout_in_sec = timeout_in_sec @@ -57,9 +57,8 @@ def _check_already_running(self): root_state_dir = get_root_state_directory() self.process_checker = ProcessChecker(root_state_dir) - if self.process_checker.already_running: - self.logger.error(f"Another Tribler instance is already using directory: {self.working_dir}") - asyncio.get_running_loop().stop() + self.process_checker.check_and_restart_if_necessary() + self.process_checker.create_lock() def _enable_graceful_shutdown(self): self.logger.info("Enabling graceful shutdown") @@ -74,6 +73,9 @@ def signal_handler(signum, frame): def _graceful_shutdown(self): self.logger.info("Shutdown gracefully") + if self.process_checker: + self.process_checker.remove_lock() + task = asyncio.create_task(self.session.shutdown()) task.add_done_callback(lambda result: asyncio.get_running_loop().stop()) diff --git a/src/tribler/gui/core_manager.py b/src/tribler/gui/core_manager.py index 5619f1cf822..dddc13e632c 100644 --- a/src/tribler/gui/core_manager.py +++ b/src/tribler/gui/core_manager.py @@ -3,6 +3,7 @@ import re import sys from pathlib import Path +from typing import Optional from PyQt5.QtCore import QObject, QProcess, QProcessEnvironment from PyQt5.QtNetwork import QNetworkRequest @@ -27,7 +28,7 @@ def __init__(self, root_state_dir: Path, api_port: int, api_key: str, app_manage self._logger = logging.getLogger(self.__class__.__name__) self.app_manager = app_manager self.root_state_dir = root_state_dir - self.core_process = None + self.core_process: Optional[QProcess] = None self.api_port = api_port self.api_key = api_key self.events_manager = events_manager diff --git a/src/tribler/gui/tribler_window.py b/src/tribler/gui/tribler_window.py index bbe36d52c4f..e5fe87ca2b0 100644 --- a/src/tribler/gui/tribler_window.py +++ b/src/tribler/gui/tribler_window.py @@ -46,7 +46,6 @@ from tribler.core.upgrade.version_manager import VersionHistory from tribler.core.utilities.network_utils import default_network_utils -from tribler.core.utilities.process_checker import ProcessChecker from tribler.core.utilities.rest_utils import ( FILE_SCHEME, MAGNET_SCHEME, @@ -1190,14 +1189,12 @@ def dropEvent(self, e): e.accept() def clicked_force_shutdown(self): - process_checker = ProcessChecker(self.root_state_dir) - if process_checker.already_running: - core_pid = process_checker.get_pid_from_lock_file() - try: - os.kill(int(core_pid), 9) - except OSError: # The core process can exit before the GUI process attempts to kill it - pass - # Stop the Qt application + pid = self.core_manager.core_process.pid() + try: + os.kill(pid, 9) + except OSError: + pass + self.app_manager.quit_application() def clicked_skip_conversion(self):