-
Notifications
You must be signed in to change notification settings - Fork 0
[codex] fix trash and runtime sync churn #27
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -7,7 +7,8 @@ import Foundation | |
| /// that a propagated cleanup or accidental `rm -rf` is recoverable. The | ||
| /// quarantine would otherwise grow without bound; this actor sweeps | ||
| /// top-level buckets whose mtime is older than `retentionDays` (default | ||
| /// 30) on a daily cadence. | ||
| /// 7) on a daily cadence, and also enforces size/count caps for high-churn | ||
| /// environments where many fresh buckets can exhaust disk before aging out. | ||
| /// | ||
| /// Janitor is intentionally conservative: | ||
| /// - Only touches the top-level UUID-named buckets under `trashRoot`. | ||
|
|
@@ -28,17 +29,26 @@ public actor TrashJanitor { | |
| // to read from outside the actor without awaiting. | ||
| public nonisolated let trashRoot: URL | ||
| public nonisolated let retentionDays: Int | ||
| public nonisolated let maxBytes: Int64 | ||
| public nonisolated let maxBucketCount: Int | ||
| public nonisolated let capMinAgeSeconds: TimeInterval | ||
| public nonisolated let sweepInterval: Duration | ||
| private nonisolated let logger: AppLogger | ||
| private let fm = FileManager.default | ||
| private var loopTask: Task<Void, Never>? | ||
|
|
||
| public init(trashRoot: URL = TrashJanitor.defaultTrashRoot(), | ||
| retentionDays: Int = 30, | ||
| retentionDays: Int = Preferences.defaultTrashRetentionDays, | ||
| maxBytes: Int64 = Preferences.defaultTrashMaxBytes, | ||
| maxBucketCount: Int = Preferences.defaultTrashMaxBuckets, | ||
| capMinAgeSeconds: TimeInterval = 60 * 60, | ||
| sweepInterval: Duration = .seconds(24 * 60 * 60), | ||
| logger: AppLogger = .shared) { | ||
| self.trashRoot = trashRoot | ||
| self.retentionDays = max(1, retentionDays) | ||
| self.maxBytes = max(0, maxBytes) | ||
| self.maxBucketCount = max(0, maxBucketCount) | ||
| self.capMinAgeSeconds = max(0, capMinAgeSeconds) | ||
| self.sweepInterval = sweepInterval | ||
| self.logger = logger | ||
| } | ||
|
|
@@ -70,6 +80,14 @@ public actor TrashJanitor { | |
| public let errors: Int | ||
| } | ||
|
|
||
| private struct Bucket: Sendable { | ||
| let url: URL | ||
| let name: String | ||
| let mtime: Date | ||
| let size: Int64 | ||
| let fileCount: Int | ||
| } | ||
|
|
||
| /// Public for tests so they can call deterministically. | ||
| @discardableResult | ||
| public func sweepOnce() async -> SweepOutcome { | ||
|
|
@@ -96,30 +114,71 @@ public actor TrashJanitor { | |
| var removed = 0 | ||
| var bytes: Int64 = 0 | ||
| var errors = 0 | ||
| var buckets: [Bucket] = [] | ||
| for url in entries { | ||
| scanned += 1 | ||
| // Defensive: only sweep UUID-named buckets we created. | ||
| guard UUID(uuidString: url.lastPathComponent) != nil else { | ||
| let name = url.lastPathComponent | ||
| guard UUID(uuidString: name) != nil else { | ||
| continue | ||
| } | ||
| let values = try? url.resourceValues(forKeys: [ | ||
| .contentModificationDateKey, .isDirectoryKey, | ||
| ]) | ||
| guard values?.isDirectory == true else { continue } | ||
| guard let mtime = values?.contentModificationDate else { continue } | ||
| guard mtime < cutoff else { continue } | ||
|
|
||
| let size = directorySize(url) ?? 0 | ||
| do { | ||
| try fm.removeItem(at: url) | ||
| removed += 1 | ||
| bytes += size | ||
| logger.info("TrashJanitor: removed \(url.lastPathComponent) (\(size) bytes, mtime \(mtime))", | ||
| category: "trash") | ||
| } catch { | ||
| errors += 1 | ||
| logger.warning("TrashJanitor: removeItem failed for \(url.path): \(error)", | ||
| category: "trash") | ||
|
|
||
| let usage = directoryUsage(url) ?? (bytes: 0, files: 0) | ||
| buckets.append(Bucket( | ||
| url: url, | ||
| name: name, | ||
| mtime: mtime, | ||
| size: usage.bytes, | ||
| fileCount: usage.files | ||
| )) | ||
| } | ||
|
|
||
| let capCutoff = Date().addingTimeInterval(-capMinAgeSeconds) | ||
| var remaining: [Bucket] = [] | ||
| for bucket in buckets { | ||
| let isExpired = bucket.mtime < cutoff | ||
| let isEmptyAndSettled = bucket.fileCount == 0 && bucket.mtime < capCutoff | ||
| if isExpired || isEmptyAndSettled { | ||
| let reason = isExpired ? "retention" : "empty" | ||
| if remove(bucket, reason: reason) { | ||
| removed += 1 | ||
| bytes += bucket.size | ||
| } else { | ||
| errors += 1 | ||
| } | ||
| } else { | ||
| remaining.append(bucket) | ||
| } | ||
| } | ||
|
|
||
| var totalBytes = remaining.reduce(Int64(0)) { $0 + $1.size } | ||
| var totalBuckets = remaining.count | ||
| let exceedsBytes = maxBytes > 0 && totalBytes > maxBytes | ||
| let exceedsBuckets = maxBucketCount > 0 && totalBuckets > maxBucketCount | ||
| if exceedsBytes || exceedsBuckets { | ||
| let candidates = remaining | ||
| .filter { $0.mtime < capCutoff } | ||
| .sorted { lhs, rhs in | ||
| if lhs.mtime == rhs.mtime { return lhs.name < rhs.name } | ||
| return lhs.mtime < rhs.mtime | ||
| } | ||
| for bucket in candidates { | ||
| let overBytes = maxBytes > 0 && totalBytes > maxBytes | ||
| let overBuckets = maxBucketCount > 0 && totalBuckets > maxBucketCount | ||
| guard overBytes || overBuckets else { break } | ||
| if remove(bucket, reason: "cap") { | ||
| removed += 1 | ||
| bytes += bucket.size | ||
| totalBytes -= bucket.size | ||
| totalBuckets -= 1 | ||
| } else { | ||
| errors += 1 | ||
| } | ||
| } | ||
| } | ||
| outcome = SweepOutcome(scanned: scanned, removed: removed, | ||
|
|
@@ -131,19 +190,34 @@ public actor TrashJanitor { | |
| return outcome | ||
| } | ||
|
|
||
| private func directorySize(_ url: URL) -> Int64? { | ||
| private func remove(_ bucket: Bucket, reason: String) -> Bool { | ||
| do { | ||
| try fm.removeItem(at: bucket.url) | ||
| logger.info("TrashJanitor: removed \(bucket.name) (\(bucket.size) bytes, mtime \(bucket.mtime), reason=\(reason))", | ||
| category: "trash") | ||
| return true | ||
| } catch { | ||
| logger.warning("TrashJanitor: removeItem failed for \(bucket.url.path): \(error)", | ||
| category: "trash") | ||
| return false | ||
| } | ||
| } | ||
|
|
||
| private func directoryUsage(_ url: URL) -> (bytes: Int64, files: Int)? { | ||
| guard let it = fm.enumerator(at: url, | ||
| includingPropertiesForKeys: [.totalFileSizeKey], | ||
| options: [.skipsHiddenFiles]) else { | ||
| options: []) else { | ||
| return nil | ||
| } | ||
| var total: Int64 = 0 | ||
| var files = 0 | ||
| for case let fileURL as URL in it { | ||
| let values = try? fileURL.resourceValues(forKeys: [.totalFileSizeKey]) | ||
| if let bytes = values?.totalFileSize { | ||
| total += Int64(bytes) | ||
| } | ||
| files += 1 | ||
| } | ||
| return total | ||
| return (total, files) | ||
| } | ||
|
Comment on lines
+206
to
222
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The private func directoryUsage(_ url: URL) -> (bytes: Int64, files: Int)? {
guard let it = fm.enumerator(at: url,
includingPropertiesForKeys: [.totalFileSizeKey, .isDirectoryKey],
options: []) else {
return nil
}
var total: Int64 = 0
var files = 0
for case let fileURL as URL in it {
let values = try? fileURL.resourceValues(forKeys: [.totalFileSizeKey, .isDirectoryKey])
if values?.isDirectory == true {
continue
}
if let bytes = values?.totalFileSize {
total += Int64(bytes)
}
files += 1
}
return (total, files)
} |
||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If
directoryUsagereturnsnil(e.g., due to transient I/O or permission errors), treating it as(bytes: 0, files: 0)will setbucket.fileCountto0. This causesisEmptyAndSettledto evaluate totrue, leading to the premature deletion of the entire bucket and potential data loss. Skipping the bucket when usage cannot be determined is a much safer approach.