Skip to content

fix(deleter): handle non-directory entries in log_root#37716

Open
Akasxh wants to merge 2 commits intocommaai:masterfrom
Akasxh:fix/deleter-handle-non-directory-entries
Open

fix(deleter): handle non-directory entries in log_root#37716
Akasxh wants to merge 2 commits intocommaai:masterfrom
Akasxh:fix/deleter-handle-non-directory-entries

Conversation

@Akasxh
Copy link
Copy Markdown

@Akasxh Akasxh commented Mar 22, 2026

Summary

  • The deleter crashed with NotADirectoryError when stray files (e.g. 2023-09-23.tar.gz) or symlinks existed in log_root, because os.listdir() was called on every entry unconditionally
  • Now checks os.path.isdir() before listing contents for lock files; files and symlinks are deleted directly with os.remove()
  • Minimal diff — no refactoring, no new abstractions, just the guard check

Tests

Added 4 new test cases covering the scenarios from the issue:

  • test_delete_stray_file — tar.gz and other non-directory files get cleaned up
  • test_delete_stray_symlink — symlinks are unlinked without following the target
  • test_delete_mixed_entries — mix of segment dirs, stray files, nested dirs, hidden files
  • test_delete_continues_after_error — deleter moves to next entry if one fails (e.g. permission error)

Test plan

  • pytest system/loggerd/tests/test_deleter.py -v passes all existing + new tests
  • Verify on device with stray files in /data/media/0/realdata/

Fixes #30102

Akasxh added 2 commits March 22, 2026 17:28
The deleter crashed with NotADirectoryError when encountering stray
files (e.g. tar.gz archives) or symlinks in the log_root directory,
because os.listdir() was called unconditionally on every entry.

Now checks if an entry is a real directory before attempting to list
its contents for lock files. Files and symlinks are deleted directly
with os.remove() instead of shutil.rmtree().

Adds tests for:
- Stray files in log_root (the original reported bug)
- Symlinks (removed without following the target)
- Mixed entries (dirs, files, nested dirs, hidden files)
- Continued deletion after per-entry errors

Fixes commaai#30102
listdir_by_creation() only returns directories, so stray files and
symlinks were never considered for deletion. Now collects non-directory
entries separately and prioritizes them for cleanup before segment dirs.
@Akasxh
Copy link
Copy Markdown
Author

Akasxh commented Mar 22, 2026

Note: this PR takes a minimal-diff approach compared to #37642 which refactors into new helper functions. The key differences:

  • Minimal changes to existing logic — no new functions, just guards around the existing loop
  • Stray files cleaned first — non-directory entries are prioritized before segment dirs, since they're foreign and shouldn't be there
  • 4 targeted test cases covering the exact scenarios from the original report (stray tar.gz files, symlinks, mixed entries, error continuation)

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.

deleter: handle non-openpilot created files

1 participant