Skip to content

Commit fc87377

Browse files
kochj23claude
andcommitted
fix: Resolve large job failures, iCloud issues, and 6 other bugs (v1.7.2)
Large jobs dying: - Cap executeCommand output buffer at 10 MB to prevent OOM on 50k+ file jobs - Change verbose/compress defaults to false (were generating hundreds of MB of output) - Add file count limit (50k) in analyzeSourceDirectory — prevents RAM exhaustion in parallel mode - Add file count limit (100k) + Task.yield() in calculateDirectoryChecksum — prevents blocking iCloud not working: - Add --exclude=*.icloud to iCloud destinations — rsync was trying to read evicted placeholder stubs - Remove Thread.sleep(0.5) from createDestinationDirectory — blocking sleep on cooperative thread Other fixes: - Convert runScript from blocking waitUntilExit() to async continuation — hung scripts no longer freeze execution - Validate rsh/rsyncPath options before use — must be absolute paths to existing executables Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
1 parent 6e0d62e commit fc87377

4 files changed

Lines changed: 105 additions & 32 deletions

File tree

RsyncGUI.xcodeproj/project.pbxproj

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -595,7 +595,7 @@
595595
CODE_SIGN_ENTITLEMENTS = "RsyncGUI Widget/RsyncGUI_Widget.entitlements";
596596
CODE_SIGN_IDENTITY = "-";
597597
CODE_SIGN_STYLE = Automatic;
598-
CURRENT_PROJECT_VERSION = 16;
598+
CURRENT_PROJECT_VERSION = 17;
599599
DEVELOPMENT_TEAM = "";
600600
GENERATE_INFOPLIST_FILE = YES;
601601
INFOPLIST_FILE = "RsyncGUI Widget/Info.plist";
@@ -607,7 +607,7 @@
607607
"@executable_path/../../../../Frameworks",
608608
);
609609
MACOSX_DEPLOYMENT_TARGET = 14.0;
610-
MARKETING_VERSION = 1.7.1;
610+
MARKETING_VERSION = 1.7.2;
611611
PRODUCT_BUNDLE_IDENTIFIER = com.jordankoch.rsyncgui.widget;
612612
PRODUCT_NAME = "$(TARGET_NAME)";
613613
SDKROOT = macosx;
@@ -625,7 +625,7 @@
625625
CODE_SIGN_ENTITLEMENTS = "RsyncGUI Widget/RsyncGUI_Widget.entitlements";
626626
CODE_SIGN_IDENTITY = "-";
627627
CODE_SIGN_STYLE = Automatic;
628-
CURRENT_PROJECT_VERSION = 16;
628+
CURRENT_PROJECT_VERSION = 17;
629629
DEVELOPMENT_TEAM = "";
630630
GENERATE_INFOPLIST_FILE = YES;
631631
INFOPLIST_FILE = "RsyncGUI Widget/Info.plist";
@@ -637,7 +637,7 @@
637637
"@executable_path/../../../../Frameworks",
638638
);
639639
MACOSX_DEPLOYMENT_TARGET = 14.0;
640-
MARKETING_VERSION = 1.7.1;
640+
MARKETING_VERSION = 1.7.2;
641641
PRODUCT_BUNDLE_IDENTIFIER = com.jordankoch.rsyncgui.widget;
642642
PRODUCT_NAME = "$(TARGET_NAME)";
643643
SDKROOT = macosx;

RsyncGUI/Models/RsyncOptions.swift

Lines changed: 17 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,8 @@ import Foundation
1111
struct RsyncOptions: Codable {
1212
// MARK: - Common Options
1313
var archive: Bool = true // -a: archive mode (recursive, preserve permissions, times, etc.)
14-
var verbose: Bool = true // -v: verbose output
15-
var compress: Bool = true // -z: compress during transfer
14+
var verbose: Bool = false // -v: verbose output (off by default — generates huge output on large jobs)
15+
var compress: Bool = false // -z: compress during transfer (off by default — slows local/LAN syncs)
1616
var delete: Bool = false // --delete: delete extraneous files from dest
1717
var dryRun: Bool = false // -n: dry run (show what would be done)
1818

@@ -289,11 +289,23 @@ struct RsyncOptions: Codable {
289289

290290
// SSH & Remote
291291
if let shell = rsh {
292-
args.append("-e")
293-
args.append(shell)
292+
// Validate rsh is an absolute path to an existing executable — prevents arbitrary program execution
293+
let expandedShell = (shell as NSString).expandingTildeInPath
294+
if expandedShell.hasPrefix("/") && !expandedShell.contains("..") && FileManager.default.isExecutableFile(atPath: expandedShell) {
295+
args.append("-e")
296+
args.append(expandedShell)
297+
} else {
298+
NSLog("[RsyncOptions] Rejected rsh value '%@' — must be absolute path to an existing executable", shell.prefix(200).description)
299+
}
294300
}
295301
if let rsyncProgram = rsyncPath {
296-
args.append("--rsync-path=\(rsyncProgram)")
302+
// Validate rsync-path contains only safe path characters (it's a remote path, so we can't check existence)
303+
let safePath = #"^[a-zA-Z0-9/_.\-]+$"#
304+
if rsyncProgram.range(of: safePath, options: .regularExpression) != nil {
305+
args.append("--rsync-path=\(rsyncProgram)")
306+
} else {
307+
NSLog("[RsyncOptions] Rejected rsync-path value '%@' — contains unsafe characters", rsyncProgram.prefix(200).description)
308+
}
297309
}
298310
if let portNum = port {
299311
args.append("--port=\(portNum)")

RsyncGUI/Services/AdvancedExecutionService.swift

Lines changed: 37 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -278,15 +278,33 @@ class AdvancedExecutionService {
278278

279279
// MARK: - Helper Methods
280280

281-
/// Analyze source directory and list all files
281+
/// Maximum number of files to load into memory for parallel splitting.
282+
/// Beyond this limit, parallel execution falls back to standard rsync (which handles large
283+
/// file trees far more efficiently than enumerating them all into a Swift array).
284+
private static let maxParallelFiles = 50_000
285+
286+
/// Analyze source directory and list all files for parallel splitting.
282287
private func analyzeSourceDirectory(path: String) async throws -> [String] {
283288
let expandedPath = path.replacingOccurrences(of: "~", with: FileManager.default.homeDirectoryForCurrentUser.path)
284289
let url = URL(fileURLWithPath: expandedPath)
285290

286291
var files: [String] = []
292+
files.reserveCapacity(min(1000, AdvancedExecutionService.maxParallelFiles))
287293

288294
if let enumerator = FileManager.default.enumerator(at: url, includingPropertiesForKeys: [.isRegularFileKey]) {
289295
for case let fileURL as URL in enumerator {
296+
// Yield periodically to avoid starving the cooperative thread pool
297+
if files.count % 5000 == 0 && files.count > 0 {
298+
await Task.yield()
299+
}
300+
301+
// Cap file list to prevent OOM on very large directories.
302+
// Standard rsync execution handles large trees better than splitting by file list.
303+
if files.count >= AdvancedExecutionService.maxParallelFiles {
304+
NSLog("[Parallel] File count hit %d limit — truncating list. Remaining files handled by rsync directly.", AdvancedExecutionService.maxParallelFiles)
305+
break
306+
}
307+
290308
if let isRegularFile = try? fileURL.resourceValues(forKeys: [.isRegularFileKey]).isRegularFile,
291309
isRegularFile {
292310
let relativePath = fileURL.path.replacingOccurrences(of: url.path + "/", with: "")
@@ -347,17 +365,34 @@ class AdvancedExecutionService {
347365
}
348366
}
349367

350-
/// Calculate directory checksum (fast hash of file list + mtimes)
368+
/// Maximum number of files to examine when calculating a directory checksum.
369+
/// Walking millions of files to detect changes is slower than just running rsync dry-run.
370+
private static let maxChecksumFiles = 100_000
371+
372+
/// Calculate directory checksum (fast hash of file list + mtimes).
373+
/// Caps at maxChecksumFiles to prevent blocking on very large directories.
351374
private func calculateDirectoryChecksum(path: String) async throws -> String {
352375
let expandedPath = path.replacingOccurrences(of: "~", with: FileManager.default.homeDirectoryForCurrentUser.path)
353376
let url = URL(fileURLWithPath: expandedPath)
354377

355378
var checksumData = Data()
379+
var fileCount = 0
356380

357381
if let enumerator = FileManager.default.enumerator(at: url, includingPropertiesForKeys: [.contentModificationDateKey, .fileSizeKey]) {
358382
var fileInfos: [(path: String, mtime: Date, size: Int64)] = []
359383

360384
for case let fileURL as URL in enumerator {
385+
// Yield every 5,000 files to avoid blocking the cooperative thread pool
386+
if fileCount % 5000 == 0 && fileCount > 0 {
387+
await Task.yield()
388+
}
389+
390+
fileCount += 1
391+
if fileCount > AdvancedExecutionService.maxChecksumFiles {
392+
NSLog("[Conditional] Checksum file limit (%d) reached — result covers partial directory", AdvancedExecutionService.maxChecksumFiles)
393+
break
394+
}
395+
361396
if let resourceValues = try? fileURL.resourceValues(forKeys: [.contentModificationDateKey, .fileSizeKey]),
362397
let mtime = resourceValues.contentModificationDate,
363398
let size = resourceValues.fileSize {

RsyncGUI/Services/RsyncExecutor.swift

Lines changed: 47 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,10 @@ import Combine
1010

1111
/// Executes rsync commands and provides real-time progress updates
1212
class RsyncExecutor: ObservableObject {
13+
14+
/// Maximum bytes of rsync output to retain in memory per execution.
15+
/// Large jobs with --verbose --progress can generate hundreds of MB — cap prevents OOM kills.
16+
private static let maxOutputBytes = 10 * 1024 * 1024 // 10 MB
1317
@Published var progress: RsyncProgress?
1418
@Published var isRunning = false
1519

@@ -275,31 +279,37 @@ class RsyncExecutor: ObservableObject {
275279
NSLog("[RsyncExecutor] Executing script: %@", expanded)
276280

277281
let process = Process()
278-
let env = [
282+
process.executableURL = URL(fileURLWithPath: expanded)
283+
process.arguments = []
284+
process.environment = [
279285
"JOB_NAME": jobName,
280286
"JOB_STATUS": status,
281287
"FILES_TRANSFERRED": String(filesTransferred),
282288
"PATH": "/usr/local/bin:/usr/bin:/bin:/usr/sbin:/sbin"
283289
]
284290

285-
process.executableURL = URL(fileURLWithPath: expanded)
286-
process.arguments = []
287-
288-
process.environment = env
289-
290291
let pipe = Pipe()
291292
process.standardOutput = pipe
292293
process.standardError = pipe
293-
defer {
294-
pipe.fileHandleForReading.closeFile()
295-
}
296294

297-
try process.run()
298-
process.waitUntilExit()
299-
300-
if process.terminationStatus != 0 {
301-
let output = String(data: pipe.fileHandleForReading.readDataToEndOfFile(), encoding: .utf8) ?? ""
302-
NSLog("[RsyncExecutor] Script failed with exit code %d: %@", process.terminationStatus, output)
295+
// Use async continuation instead of waitUntilExit() — the blocking call would
296+
// occupy a cooperative thread pool thread for the entire script duration.
297+
try await withCheckedThrowingContinuation { (continuation: CheckedContinuation<Void, Error>) in
298+
process.terminationHandler = { proc in
299+
let outputData = pipe.fileHandleForReading.readDataToEndOfFile()
300+
pipe.fileHandleForReading.closeFile()
301+
if proc.terminationStatus != 0 {
302+
let output = String(data: outputData, encoding: .utf8) ?? ""
303+
NSLog("[RsyncExecutor] Script failed with exit code %d: %@", proc.terminationStatus, output)
304+
}
305+
continuation.resume()
306+
}
307+
do {
308+
try process.run()
309+
} catch {
310+
pipe.fileHandleForReading.closeFile()
311+
continuation.resume(throwing: RsyncError.executionFailed(error))
312+
}
303313
}
304314
}
305315

@@ -388,10 +398,10 @@ class RsyncExecutor: ObservableObject {
388398
try FileManager.default.createDirectory(atPath: expandedPath, withIntermediateDirectories: true, attributes: nil)
389399
print("[RsyncExecutor] ✅ Created destination directory: \(expandedPath)")
390400

391-
// For iCloud Drive, wait a moment for iCloud to recognize the new folder
392-
if expandedPath.contains("com~apple~CloudDocs") {
393-
Thread.sleep(forTimeInterval: 0.5)
394-
}
401+
// Note: iCloud's daemon (bird) will pick up the new folder asynchronously.
402+
// A blocking sleep here is both incorrect and unnecessary — rsync handles
403+
// the destination directory being present at the time it runs.
404+
NSLog("[RsyncExecutor] ✅ Created iCloud destination directory: %@", expandedPath)
395405
} catch {
396406
print("[RsyncExecutor] ❌ Failed to create directory: \(error.localizedDescription)")
397407
print("[RsyncExecutor] Path: \(expandedPath)")
@@ -465,6 +475,14 @@ class RsyncExecutor: ObservableObject {
465475
args.append(expandedSource)
466476
}
467477

478+
// iCloud: exclude .icloud placeholder stubs.
479+
// Files evicted from local storage appear as ".filename.icloud" — rsync cannot read them
480+
// and will either error out or transfer the stub file, not the real content.
481+
if dest.type == .iCloudDrive {
482+
args.append("--exclude=*.icloud")
483+
NSLog("[RsyncExecutor] iCloud destination: added --exclude=*.icloud to skip offloaded file placeholders")
484+
}
485+
468486
// Local/iCloud destination
469487
var expandedDest = dest.path.replacingOccurrences(of: "~", with: FileManager.default.homeDirectoryForCurrentUser.path)
470488

@@ -506,6 +524,7 @@ class RsyncExecutor: ObservableObject {
506524
var outputData = Data()
507525
var errorData = Data()
508526
var isTerminating = false
527+
var outputTruncated = false
509528

510529
// Track active handlers to prevent race conditions
511530
let handlerGroup = DispatchGroup()
@@ -530,9 +549,16 @@ class RsyncExecutor: ObservableObject {
530549
// Make a defensive copy of the data for thread safety
531550
let dataCopy = Data(data)
532551

533-
// Thread-safe append
552+
// Thread-safe append with output cap to prevent OOM on large jobs
534553
dataLock.lock()
535-
outputData.append(dataCopy)
554+
if outputData.count < RsyncExecutor.maxOutputBytes {
555+
outputData.append(dataCopy)
556+
} else if !outputTruncated {
557+
outputTruncated = true
558+
let notice = Data("\n⚠️ Output truncated at 10 MB. Full output available in system log (Console.app).\n".utf8)
559+
outputData.append(notice)
560+
NSLog("[RsyncExecutor] Output cap reached (10 MB) — truncating display output")
561+
}
536562
dataLock.unlock()
537563

538564
if let output = String(data: dataCopy, encoding: .utf8) {

0 commit comments

Comments
 (0)