warnings bookkeeping: count warning codes instead of keeping the warnings

src/borg/archiver/__init__.py

@@ -147,7 +147,7 @@ def print_warning(self, msg, *args, **kw):
         assert warning_type in ("percent", "curly")
         warning_msgid = kw.get("msgid")
         if warning_code is not None:
-            add_warning(msg, *args, wc=warning_code, wt=warning_type)
+            add_warning(warning_code)
         if warning_type == "percent":
             output = args and msg % args or msg
         else:  # == "curly"

src/borg/helpers/__init__.py

@@ -7,7 +7,7 @@
 
 import os
 import logging
-from collections import namedtuple
+from collections import Counter
 
 from ..constants import *  # NOQA
 
@@ -80,22 +80,20 @@
 workarounds = tuple(os.environ.get("BORG_WORKAROUNDS", "").split(","))
 
 
-# element data type for warnings_list:
-warning_info = namedtuple("warning_info", "wc,msg,args,wt")
+# The warnings emitted while borg is running only influence the final exit code (see get_ec), thus all
+# we keep about them is how many there were per warning code (a handful of distinct codes at most).
+# Do not store the warning messages or their args here: for a BackupWarning, the args include the
+# caught exception, and 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 - e.g. one chunk per failed file once the disk
+# borg extracts to is full.
+_warning_counts: Counter[int] = Counter()  # warning code -> number of warnings with that code
 
-"""
-The global warnings_list variable is used to collect warning_info elements while Borg is running.
-"""
-_warnings_list: list[warning_info] = []
 
-
-def add_warning(msg, *args, **kwargs):
-    global _warnings_list
-    warning_code = kwargs.get("wc", EXIT_WARNING)
-    assert isinstance(warning_code, int)
-    warning_type = kwargs.get("wt", "percent")
-    assert warning_type in ("percent", "curly")
-    _warnings_list.append(warning_info(warning_code, msg, args, warning_type))
+def add_warning(wc=EXIT_WARNING):
+    """Record that a warning with warning code wc was emitted, see get_ec."""
+    assert isinstance(wc, int)
+    _warning_counts[wc] += 1
 
 
 """
@@ -149,15 +147,13 @@ def set_ec(ec):
     _exit_code = max_ec(_exit_code, ec)
 
 
-def init_ec_warnings(ec=EXIT_SUCCESS, warnings=None):
+def init_ec_warnings(ec=EXIT_SUCCESS):
     """
-    (Re-)initialize the globals for the exit code and the warnings list.
+    (Re-)initialize the globals for the exit code and the warning counts.
     """
-    global _exit_code, _warnings_list
+    global _exit_code
     _exit_code = ec
-    warnings = [] if warnings is None else warnings
-    assert isinstance(warnings, list)
-    _warnings_list = warnings
+    _warning_counts.clear()
 
 
 def get_ec(ec=None):
@@ -173,13 +169,12 @@ def get_ec(ec=None):
         # there was a signal/error/warning, return its exit code
         return _exit_code
     assert exit_code_class == "success"
-    global _warnings_list
-    if not _warnings_list:
-        # we do not have any warnings in warnings list, return success exit code
+    if not _warning_counts:
+        # we do not have any warnings, return success exit code
         return _exit_code
     # looks like we have some warning(s)
-    rcs = sorted({w_info.wc for w_info in _warnings_list})
-    logger.debug(f"rcs: {rcs!r}")
+    rcs = sorted(_warning_counts)
+    logger.debug(f"warning counts: {dict(sorted(_warning_counts.items()))!r}")
     if len(rcs) == 1:
         # easy: there was only one kind of warning, so we can be specific
         return rcs[0]

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(EXIT_WARNING_BASE + 1)
+    add_warning(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(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(EXIT_WARNING_BASE + 1)
+    assert get_reset_ec() == EXIT_WARNING_BASE + 1
+    assert get_ec() == EXIT_SUCCESS