diff --git a/src/borg/helpers/__init__.py b/src/borg/helpers/__init__.py index b3d5216d51..daeab865b0 100644 --- a/src/borg/helpers/__init__.py +++ b/src/borg/helpers/__init__.py @@ -95,6 +95,11 @@ def add_warning(msg, *args, **kwargs): assert isinstance(warning_code, int) warning_type = kwargs.get("wt", "percent") assert warning_type in ("percent", "curly") + # Store exceptions given as args (e.g. the BackupError wrapped by a BackupWarning) as their message text: + # an exception references its traceback and thus the frames (with all their locals, e.g. the chunk data + # that was being written) of the code that failed - keeping that for every warning, for the whole borg + # run, would leak memory. The text is what formatting the message with the exception gives anyway. + args = tuple(str(arg) if isinstance(arg, BaseException) else arg for arg in args) _warnings_list.append(warning_info(warning_code, msg, args, warning_type)) diff --git a/src/borg/testsuite/archiver/return_codes_test.py b/src/borg/testsuite/archiver/return_codes_test.py index 6e22ff4937..21ca05b341 100644 --- a/src/borg/testsuite/archiver/return_codes_test.py +++ b/src/borg/testsuite/archiver/return_codes_test.py @@ -1,7 +1,13 @@ +import errno +import gc import os +import weakref +from ...archiver import Archiver from ...constants import * # NOQA -from ...helpers import IncludePatternNeverMatchedWarning +from ...helpers import IncludePatternNeverMatchedWarning, BackupError, BackupOSError, BackupWarning +from ...helpers import get_reset_ec, init_ec_warnings, modern_ec +from ...logger import setup_logging from ...repository import Repository from . import cmd, changedir, generate_archiver_tests # NOQA @@ -33,3 +39,26 @@ def test_exit_codes(archivers, request, monkeypatch): cmd(archiver, "create", "archive", "input", fork=True, exit_code=EXIT_ERROR) monkeypatch.setenv("BORG_EXIT_CODES", "modern") cmd(archiver, "create", "archive", "input", fork=True, exit_code=Repository.DoesNotExist.exit_mcode) + + +def test_print_warning_instance_does_not_retain_exception(): + """The warnings bookkeeping for the final exit code must not keep the wrapped exception alive. + + An exception references its traceback and thus the frames (with all their locals, e.g. the chunk + data that was being written) of the code that failed - keeping that per warning would leak memory. + """ + setup_logging() + init_ec_warnings() + archiver = Archiver() + try: + raise BackupOSError("write", OSError(errno.ENOSPC, "No space left on device")) + except BackupError as exc: + exc_ref = weakref.ref(exc) + archiver.print_warning_instance(BackupWarning("input/file", exc)) + # "except ... as exc" unbinds exc when its block ends, so the exception is only still alive if the + # warnings bookkeeping references it. CPython frees it right away (refcounting), PyPy only when + # its GC runs, so collect explicitly before looking at the weakref. + gc.collect() + assert exc_ref() is None + # the warning was recorded for the exit code, though. + assert get_reset_ec() == (BackupOSError.exit_mcode if modern_ec else EXIT_WARNING) diff --git a/src/borg/testsuite/helpers/__init__test.py b/src/borg/testsuite/helpers/__init__test.py index 4c01179293..24fe10fd11 100644 --- a/src/borg/testsuite/helpers/__init__test.py +++ b/src/borg/testsuite/helpers/__init__test.py @@ -1,7 +1,7 @@ import pytest from ...constants import * # NOQA -from ...helpers import classify_ec, max_ec +from ...helpers import classify_ec, max_ec, add_warning, get_ec, get_reset_ec, init_ec_warnings @pytest.mark.parametrize( @@ -62,3 +62,24 @@ def test_ec_invalid(): ) def test_max_ec(ec1, ec2, ec_max): assert max_ec(ec1, ec2) == ec_max + + +def test_get_ec_warnings(): + init_ec_warnings() + # no warnings: the exit code set via set_ec (or given to get_ec) is returned as is. + assert get_ec() == EXIT_SUCCESS + # only warnings of one kind: the exit code is that specific warning code. + add_warning("some warning", wc=EXIT_WARNING_BASE + 1) + add_warning("some warning", wc=EXIT_WARNING_BASE + 1) + assert get_ec() == EXIT_WARNING_BASE + 1 + # warnings of different kinds: the exit code is the generic warning code. + add_warning("another warning", wc=EXIT_WARNING_BASE + 2) + assert get_ec() == EXIT_WARNING + # an error is more severe than any warning. + assert get_ec(EXIT_ERROR) == EXIT_ERROR + # get_reset_ec returns the exit code and then starts over (no exit code, no warnings). + assert get_reset_ec() == EXIT_ERROR + assert get_ec() == EXIT_SUCCESS + add_warning("some warning", wc=EXIT_WARNING_BASE + 1) + assert get_reset_ec() == EXIT_WARNING_BASE + 1 + assert get_ec() == EXIT_SUCCESS