Skip to content

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

Merged
ThomasWaldmann merged 1 commit into
borgbackup:masterfrom
ThomasWaldmann:warnings-no-retain
Sep 8, 2026
Merged

add_warning: store exceptions given as args as text, not the exception#10331
ThomasWaldmann merged 1 commit into
borgbackup:masterfrom
ThomasWaldmann:warnings-no-retain

Conversation

@ThomasWaldmann

@ThomasWaldmann ThomasWaldmann commented Sep 6, 2026

Copy link
Copy Markdown
Member

What

For a BackupWarning, the args of the warning include the caught BackupError exception. Storing that in the global warnings list (add_warning()) kept the exception - and thus its traceback and the frames (with all their locals) of the code that failed - alive for the rest of the borg run, one per warning. For a failed file in borg extract, that is the extract_item() frame including the chunk data that was being written; on a full disk, every remaining file adds another chunk.

Measured with borg extract of 300 files of 2.5 MB whose writes all fail (ulimit -f, so each write gets EFBIG):

max RSS
master 876 MiB (300 exception objects and 555 MiB of chunk bytes kept alive via their frames)
this PR 426 MiB (same as a normal extraction of that archive)

How

add_warning() stores exceptions given as args as their message text (str(exc)), which is what formatting the warning message with the exception gives anyway. Everything else (the warning_info list, init_ec_warnings(), get_ec()) is unchanged.

Tests

  • regression test at the print_warning_instance() level: the exception wrapped by the BackupWarning must be freed afterwards (checked via weakref, after a gc.collect() for PyPy). Fails on master, passes here.
  • unit test for the get_ec() warnings logic (one kind of warning, mixed kinds, error precedence, reset)

Note: 1.4-maint has the same add_warning() in helpers/__init__.py, so the same fix applies there.

🤖 Generated with Claude Code

@codecov

codecov Bot commented Sep 6, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 87.80%. Comparing base (a6c45bc) to head (366db25).
⚠️ Report is 18 commits behind head on master.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@            Coverage Diff             @@
##           master   #10331      +/-   ##
==========================================
+ Coverage   87.72%   87.80%   +0.07%     
==========================================
  Files         103      103              
  Lines       18816    18828      +12     
  Branches     2904     2906       +2     
==========================================
+ Hits        16507    16531      +24     
+ Misses       1605     1598       -7     
+ Partials      704      699       -5     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

For a BackupWarning, the args of the warning include the caught BackupError
exception. Storing that in the global warnings list kept the exception - and
thus its traceback and the frames (with all their locals) of the code that
failed - alive for the rest of the borg run, one per warning: for a failed
file in borg extract, that is the extract_item frame including the chunk
data that was being written. borg extract of 300 files of 2.5 MB whose
writes all fail peaked at 876 MiB RSS instead of 409 MiB, and on a full
disk every remaining file adds another chunk.

Store the exception's message text instead, which is what formatting the
warning message with the exception gives anyway.

Add a regression test (the exception wrapped by a BackupWarning must be
freed after print_warning_instance()) and a unit test for the get_ec()
warnings logic.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@ThomasWaldmann ThomasWaldmann changed the title warnings bookkeeping: count warning codes instead of keeping the warnings add_warning: store exceptions given as args as text, not the exception Sep 8, 2026
@ThomasWaldmann
ThomasWaldmann merged commit 55a0997 into borgbackup:master Sep 8, 2026
23 checks passed
@ThomasWaldmann
ThomasWaldmann deleted the warnings-no-retain branch September 8, 2026 18:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant