add_warning: store exceptions given as args as text, not the exception

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

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)

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