Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixes 1-Click release for 7.11 #6568

Merged
merged 12 commits into from
Nov 20, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion build/win/makedist_win.bat
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ REM Arno: When adding files here, make sure tribler.nsi actually
REM packs them in the installer .EXE

python3 -m pip install --upgrade --no-deps --force-reinstall pydantic --no-binary pydantic
python3 -m pip install --upgrade --no-deps --force-reinstall typing_extensions --no-binary typing_extensions
python3 -m pip install --upgrade --no-deps --force-reinstall typing_extensions==3.10.0.2 --no-binary typing_extensions
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Without --no-binary PyInstaller does not include this module into the Tribler exe file on Windows
The latest version of typing_extension (4.0.0) is currently not working as well.
After #6534 we can move it into requirements with --no-binary specification


%PYTHONHOME%\Scripts\pyinstaller.exe tribler.spec

Expand Down
13 changes: 10 additions & 3 deletions src/run_tribler.py
Original file line number Diff line number Diff line change
@@ -1,18 +1,22 @@
# A fix for "LookupError: unknown encoding: idna" error.
# Adding encodings.idna to hiddenimports is not enough.
# https://github.com/pyinstaller/pyinstaller/issues/1113
import encodings.idna # pylint: disable=unused-import
import logging.config
import os
import sys

from PyQt5.QtCore import QSettings

from tribler_common.sentry_reporter.sentry_reporter import SentryReporter, SentryStrategy
from tribler_common.sentry_reporter.sentry_scrubber import SentryScrubber

logger = logging.getLogger(__name__)
CONFIG_FILE_NAME = 'triblerd.conf'

# pylint: disable=import-outside-toplevel, ungrouped-imports

def init_sentry_reporter():
from tribler_common.sentry_reporter.sentry_reporter import SentryReporter, SentryStrategy
from tribler_common.sentry_reporter.sentry_scrubber import SentryScrubber

""" Initialise sentry reporter

We use `sentry_url` as a URL for normal tribler mode and TRIBLER_TEST_SENTRY_URL
Expand Down Expand Up @@ -135,7 +139,10 @@ def init_boot_logger():
logger.info("Shutting down Tribler")
if trace_logger:
trace_logger.close()

# Flush all the logs to make sure it is written to file before it exits
for handler in logging.getLogger().handlers:
handler.flush()

SentryReporter.global_strategy = SentryStrategy.SEND_SUPPRESSED
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Without this Sentry suppression, it sometimes happens that an error takes place during the Tribler shutdown. Then Sentry tries to display the dialog window, but the Qt state is already partially destructed, which leads to mysterious crashes.

raise
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,7 @@ def scrub_event(self, event):
ThreadingIntegration(propagate_hub=True),
],
before_send=SentryReporter._before_send,
ignore_errors=[KeyboardInterrupt],
)

ignore_logger(SentryReporter._sentry_logger_name)
Expand Down
10 changes: 8 additions & 2 deletions src/tribler-core/tribler_core/check_os.py
Original file line number Diff line number Diff line change
Expand Up @@ -89,8 +89,14 @@ def should_kill_other_tribler_instances(root_state_dir):
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)
logger.info(f'Old process status: {old_process.status()}')
if old_process.status() == psutil.STATUS_ZOMBIE:
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sometimes Tribler crashes on psutil.NoSuchProcess exception when reading old_process.status()

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
Expand Down
21 changes: 12 additions & 9 deletions src/tribler-core/tribler_core/start_core.py
Original file line number Diff line number Diff line change
Expand Up @@ -174,12 +174,15 @@ def start_tribler_core(base_path, api_port, api_key, root_state_dir, gui_test_mo
loop = asyncio.get_event_loop()
loop.set_exception_handler(CoreExceptionHandler.unhandled_error_observer)

loop.run_until_complete(core_session(config, components=list(components_gen(config))))

if trace_logger:
trace_logger.close()

process_checker.remove_lock_file()
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We must always remove the lock file on Tribler crash to re-run the application tester reliably, so try/finally block is necessary here.

Copy link
Contributor Author

@kozlovsky kozlovsky Nov 19, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Our lock file mechanics are broken, as it was built on the assumption that OS does not reuse PIDs before system restart. This is not true, Windows especially reuses process IDs quickly, and that leads to many non-obvious bugs (like, Tribler is trying to kill some innocent unrelated process, assuming this is the previous Tribler core). I will open a separate issue for that.

# Flush the logs to the file before exiting
for handler in logging.getLogger().handlers:
handler.flush()
try:
loop.run_until_complete(core_session(config, components=list(components_gen(config))))
finally:
if trace_logger:
trace_logger.close()

logger.info('Remove lock file')
process_checker.remove_lock_file()

# Flush the logs to the file before exiting
for handler in logging.getLogger().handlers:
handler.flush()
38 changes: 36 additions & 2 deletions src/tribler-core/tribler_core/tests/test_check_os.py
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
from unittest.mock import patch
from unittest.mock import Mock, patch

import psutil

import pytest

from tribler_core.check_os import error_and_exit
from tribler_core.check_os import error_and_exit, should_kill_other_tribler_instances

pytestmark = pytest.mark.asyncio

Expand All @@ -14,3 +16,35 @@ async def test_error_and_exit(mocked_show_system_popup, mocked_sys_exit):
error_and_exit('title', 'text')
mocked_show_system_popup.assert_called_once_with('title', 'text')
mocked_sys_exit.assert_called_once_with(1)


@patch('tribler_core.check_os.logger.info')
@patch('sys.argv', [])
@patch('tribler_core.check_os.get_existing_tribler_pid', Mock(return_value=100))
@patch('os.getpid', Mock(return_value=200))
@patch('psutil.Process', Mock(return_value=Mock(status=Mock(side_effect=psutil.NoSuchProcess(100)))))
def test_should_kill_other_tribler_instances_process_not_found(
mocked_logger_info: Mock
):
root_state_dir = Mock()
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', Mock(return_value=100))
@patch('os.getpid', Mock(return_value=200))
@patch('psutil.Process', Mock(return_value=Mock(status=Mock(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: Mock,
mocked_kill_tribler_process: Mock,
mocked_logger_info: Mock,
):
root_state_dir = Mock()
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()
9 changes: 8 additions & 1 deletion src/tribler-gui/tribler_gui/core_manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,12 @@ def on_core_read_ready(self):
if b'Traceback' in raw_output:
self.core_traceback = decoded_output
self.core_traceback_timestamp = int(round(time.time() * 1000))
print(decoded_output.strip()) # noqa: T001
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sometimes Tribler crashes when trying to write data to stdout during the shutdown

try:
print(decoded_output.strip()) # noqa: T001
except OSError:
# Possible reason - cannot write to stdout as it was already closed during the application shutdown
if not self.shutting_down:
raise

def on_core_finished(self, exit_code, exit_status):
if self.shutting_down and self.should_stop_on_shutdown:
Expand Down Expand Up @@ -197,7 +202,9 @@ def on_received_state(self, state):
raise RuntimeError(state['last_exception'])

def stop(self, stop_app_on_shutdown=True):
self._logger.info("Stopping Core manager")
if self.core_process or self.is_core_running:
self._logger.info("Sending shutdown request to Tribler Core")
self.events_manager.shutting_down = True
TriblerNetworkRequest("shutdown", lambda _: None, method="PUT", priority=QNetworkRequest.HighPriority)

Expand Down
6 changes: 5 additions & 1 deletion src/tribler-gui/tribler_gui/error_handler.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
import traceback

from tribler_common.reported_error import ReportedError
from tribler_common.sentry_reporter.sentry_reporter import SentryReporter
from tribler_common.sentry_reporter.sentry_reporter import SentryReporter, SentryStrategy

# fmt: off
from tribler_gui.dialogs.feedbackdialog import FeedbackDialog
Expand All @@ -26,6 +26,10 @@ def gui_error(self, *exc_info):
return

info_type, info_error, tb = exc_info
if SentryReporter.global_strategy == SentryStrategy.SEND_SUPPRESSED:
self._logger.info(f'GUI error was suppressed and not sent to Sentry: {info_type.__name__}: {info_error}')
return

if info_type in self._handled_exceptions:
return
self._handled_exceptions.add(info_type)
Expand Down
16 changes: 16 additions & 0 deletions src/tribler-gui/tribler_gui/tests/test_core_manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,3 +33,19 @@ async def test_on_core_finished_raises_error():

with pytest.raises(CoreCrashedError):
core_manager.on_core_finished(exit_code=1, exit_status='exit status')


@patch('tribler_gui.core_manager.EventRequestManager', new=MagicMock())
@patch('builtins.print', MagicMock(side_effect=OSError()))
def test_on_core_read_ready():
# test that OSError on writing to stdout is suppressed during shutting down

core_manager = CoreManager(MagicMock(), MagicMock(), MagicMock())
core_manager.core_process = MagicMock(read_all=MagicMock(return_value=''))

with pytest.raises(OSError):
core_manager.on_core_read_ready()

core_manager.shutting_down = True
# no exception during shutting down
core_manager.on_core_read_ready()
12 changes: 12 additions & 0 deletions src/tribler-gui/tribler_gui/tests/test_error_handler.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import pytest

from tribler_common.reported_error import ReportedError
from tribler_common.sentry_reporter.sentry_reporter import SentryStrategy

from tribler_gui.error_handler import ErrorHandler
from tribler_gui.exceptions import CoreConnectTimeoutError, CoreCrashedError
Expand Down Expand Up @@ -31,6 +32,17 @@ async def test_gui_error_tribler_stopped(mocked_feedback_dialog: MagicMock, erro
mocked_feedback_dialog.assert_not_called()


@patch('tribler_gui.error_handler.FeedbackDialog')
@patch('tribler_common.sentry_reporter.sentry_reporter.SentryReporter.global_strategy', SentryStrategy.SEND_SUPPRESSED)
async def test_gui_error_suppressed(mocked_feedback_dialog: MagicMock, error_handler: ErrorHandler):
logger_info_mock = MagicMock()
error_handler._logger = MagicMock(info=logger_info_mock)
error_handler.gui_error(AssertionError, AssertionError('error_text'), None)
mocked_feedback_dialog.assert_not_called()
assert not error_handler._handled_exceptions
logger_info_mock.assert_called_with('GUI error was suppressed and not sent to Sentry: AssertionError: error_text')


@patch('tribler_gui.error_handler.FeedbackDialog')
async def test_gui_info_type_in_handled_exceptions(mocked_feedback_dialog: MagicMock, error_handler: ErrorHandler):
# test that if exception type in _handled_exceptions then FeedbackDialog is not called
Expand Down
1 change: 1 addition & 0 deletions src/tribler-gui/tribler_gui/tribler_window.py
Original file line number Diff line number Diff line change
Expand Up @@ -1072,6 +1072,7 @@ def show_force_shutdown():
self.gui_settings.setValue("size", self.size())

if self.core_manager.use_existing_core:
self._logger.info("Quitting Tribler GUI without stopping Tribler Core")
# Don't close the core that we are using
QApplication.quit()

Expand Down
11 changes: 6 additions & 5 deletions tribler.spec
Original file line number Diff line number Diff line change
Expand Up @@ -93,18 +93,19 @@ hiddenimports = [
'csv',
'dataclasses', # https://github.com/pyinstaller/pyinstaller/issues/5432
'ecdsa',
'pyaes',
'PIL',
'scrypt', '_scrypt',
'sqlalchemy', 'sqlalchemy.ext.baked', 'sqlalchemy.ext.declarative',
'pkg_resources', # 'pkg_resources.py2_warn', # Workaround PyInstaller & SetupTools, https://github.com/pypa/setuptools/issues/1963
'requests',
'pyaes',
'pydantic',
'PyQt5.QtTest',
'pyqtgraph',
'pyqtgraph.graphicsItems.PlotItem.plotConfigTemplate_pyqt5',
'pyqtgraph.graphicsItems.ViewBox.axisCtrlTemplate_pyqt5',
'pyqtgraph.imageview.ImageViewTemplate_pyqt5',
'PyQt5.QtTest',
'requests',
'scrypt', '_scrypt',
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe pyaes, sqlalchemy and script are not required anymore. These were dependencies for bitcoinlib (yes, we used to have a Bitcoin wallet in Tribler 😁 ) but we remove that library and we apparently forgot to remove them from tribler.spec.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good to know! I'll remove them in a separate PR after additional testing

'sqlalchemy', 'sqlalchemy.ext.baked', 'sqlalchemy.ext.declarative',
'typing_extensions',
] + widget_files + pony_deps + get_sentry_hooks()

# https://github.com/pyinstaller/pyinstaller/issues/5359
Expand Down