Merge pull request #6941 from drew2a/fix/6874

ProcessChecker refactoring

.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

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")
 

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.

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()

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()