From b26169496af0dd0ac860dddaf5f9fd792ed4261e Mon Sep 17 00:00:00 2001 From: Ocean572 <45051223+Ocean572@users.noreply.github.com> Date: Thu, 5 Nov 2026 21:54:48 -0600 Subject: [PATCH] Indexer: mark oversized pictures before decoding, tick the watchdog faster, and note the file a crash died on MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Large RAWs and layered TIFFs ran the indexing helper's reading past its memory and killed it — Swift's allocation trap, "indexer: exited (5)" — faster than the watchdog's once-a-second tick could note the file. The file stayed at the queue's head, and every restart took it first and died on it again: the #182 report has 55,000 files waiting behind one. A picture whose header says its reading would ask for more than the helper's memory limit is now marked "couldn't be read" before decoding: RAW at 192 bytes per pixel (measured 17 B/px on a real 45.44 MP Nikon Z9 NEF on macOS 26.5.1, 782 MB peak, and ≥176 in the field at an 8 GB watchdog — 192 chosen above the field floor, disclosed conservative for modern ImageIO), layered TIFFs at 16 B/px and the stb side at 8 B/px (design estimates, disclosed as unmeasured in the code). Plain TIFFs, JPEGs and PNGs keep no preflight: their scaled decode streams in tiles, measured at 70 MB for a 2,125 MP LZW TIFF and 18 MB for a 100 MP PNG. The watchdog now looks ten times a second. A helper killed on the spot writes a note beside the index from a signal handler (syscalls only, path fixed at install), and the next helper marks the file the crash was on. The Mac app and the Tauri shell treat a fatal-signal exit like a watchdog stop (restart, cap), and the 4-stops-in-10-min cap resets once the queue has moved 100 files between stops, so an index full of oversized RAWs works through them instead of freezing. Which file a crash was on is the hard half, and the pre-push review of this branch pointed out three places where the first version got it wrong: * A note with no path used to mean "mark the queue's head", which it is not: media runs in batches of eight and the whole batch stays pending till the batch ends, so a trap on the third image failed an innocent first one and left the poison queued (7 innocents per poison, with 4 as the restart cap — the queue froze before the bad file reached the head). The note is therefore still pathless (the crash beats `Indexer.nowReading`), and **the app names the file for the next helper**: both apps already see every `file` event, so `Engine` / the shell keep the last one reported and pass it as `DIGUP_STOPPED_FILE` on the restart; the head-mark stays as the fallback for a spawn with no name (a terminal run, a crash before any file event). * A fatal-signal exit was classified from the exit code alone, which is inert on Linux: `ExitStatus::code()` is None for a child killed by a signal, so the shell saw 0 and restarted nothing. The `ended` event now carries the signal (`ExitStatusExt::signal()`, Unix-only; Windows has no signals and carries the class as the SEH exit code — breakpoint, access violation, stack overflow, fast fail, illegal instruction, divide-by-zero, all matched), the Mac list gained SIGBUS (10), and the classification is a small function with unit tests in both spellings. * The preflight itself could trap: the layered-TIFF sizes come from 4-byte LONG tags, and a header saying 0xFFFFFFFF overflowed the parser's own `Int` sum — a trap inside the guard built to stop traps. Sizes above 1<<24 are garbage and get no estimate; the sums are `UInt64` and saturating on both platforms, and the budget comparisons no longer force `Int(budget)` (an absurd `--memory-limit` trapped in the helper's own setup too, via `<< 20`). Measured on this box (Apple M1, 8 GB, macOS 26.5.1): the Z9 NEF is marked in 0.17 s under the 4 GB default limit ("needed an estimated 8.1 GB ... a RAW picture of 45 MP") and decodes in 5.9 s at a 16 GB limit; a queue of NEF+HEIC+JPEG ends 2 done 1 failed with exit 0; a helper killed with SIGTRAP mid-run is followed by a helper that marks the file and finishes the rest (19 done, pending 0). Named marking, end to end against the real helper on a sandbox index of three queued text files with the newest (the queue's head) a.txt: the note alone marks a.txt (the fallback, `state='failed'`, event `stopped` naming it); the note plus `DIGUP_STOPPED_FILE=c.txt` marks **c.txt** and leaves a.txt pending — the innocent head is not failed and the queue moves past the poison. Suite: 138 green (base 125, +13, all of them the new `PreflightTests` suite: 4 layered-TIFF parser tests including the garbage-dimensions case, 3 preflight-through-ImageIO tests, 2 camera-file tests gated on `DIGUP_TEST_RAW`, 3 stopped-file note tests, 1 wording test). `cargo test` 17 green (base 13, +2 crash-class tests, +2 `reap` tests on real children: SIGTRAP gives `(None, Some(5))`, `exit 75` gives `(Some(75), None)`), and `cargo check --tests` is clean. Disclosed, as written in the code: the layered-TIFF and stb bytes-per-pixel coefficients are design estimates, not field measurements; the Windows `CrashNotes` net covers only the SEH classes the C runtime translates there (SEGV/FPE/ILL), so a Windows crash can leave no note — the fallback then marks nothing and the cap bounds the restarts, and the Windows-side blocks (the SEH codes, the `fatal_signal` no-op) compile only for Windows targets, so this box's runs do not exercise them; installing plain signal handlers trades Swift's own crash print for the note (macOS keeps its `.ips` via the re-raise, Linux loses the print, and chaining Swift's handler through `sigaction` is the natural next step); the preflight tests share the helper's global budget (`ImageExtractor.decodeMemoryBudget`) and run serialized, coupled to one another by design; and the GUI paths — the Mac app restarting a helper after a real crash, the shell app on Windows — are not exercised on this box, which runs the helpers and the unit tests, not the Tauri app. --- Sources/DigUpApp/Engine.swift | 48 +++- Sources/DigUpKit/Extractors.swift | 114 +++++++- Sources/DigUpKit/Model.swift | 37 +++ .../Platform/Apple/Extractors+Apple.swift | 34 +++ .../Platform/Portable/Images+Portable.swift | 28 +- .../Platform/Portable/Media+Portable.swift | 11 +- Sources/digup/CrashNotes.swift | 69 +++++ Sources/digup/IndexService.swift | 30 ++- Sources/digup/Watchdog.swift | 37 +-- Sources/digup/main.swift | 13 +- Tests/DigUpKitTests/PreflightTests.swift | 251 ++++++++++++++++++ shell/src-tauri/src/helper.rs | 63 ++++- shell/src-tauri/src/main.rs | 99 ++++++- 13 files changed, 777 insertions(+), 57 deletions(-) create mode 100644 Sources/digup/CrashNotes.swift create mode 100644 Tests/DigUpKitTests/PreflightTests.swift diff --git a/Sources/DigUpApp/Engine.swift b/Sources/DigUpApp/Engine.swift index 15d7d53a..f225080c 100644 --- a/Sources/DigUpApp/Engine.swift +++ b/Sources/DigUpApp/Engine.swift @@ -110,6 +110,24 @@ nonisolated final class Engine: @unchecked Sendable { private var watchdogStops: [Date] = [] /// The helper's exit status when its watchdog ended it on a file (`digup`'s Watchdog: EX_TEMPFAIL). private static let stoppedOnFile: Int32 = 75 + /// The queue's pending-file count as the helper last reported it, and what it was at the last stop on a + /// file: stops are a stall only when the queue isn't moving between them (issue #182 — 55,000 files + /// behind oversized RAWs, where the 4-stops cap froze the index after 4 of them). + private var lastPending: Int? + private var pendingAtLastStop: Int? + /// Exits that mean "the helper died on the file it was reading": fatal signals — SIGILL, SIGTRAP (Swift's + /// allocation trap, the "exited (5)" of #182), SIGABRT, SIGFPE, SIGKILL, SIGBUS (mapped-file faults: + /// the crash class a truncated picture gives), SIGSEGV. `terminationStatus` carries the signal on + /// macOS; the helper's `CrashNotes` leaves a note beside the index, so the next helper marks that + /// file. (10 was missing from the list when the pre-push review checked it: SIGBUS crashes left the + /// note unread and the helper unrestarted.) + static func diedOnFile(_ status: Int32) -> Bool { [4, 5, 6, 8, 9, 10, 11].contains(status) } + /// The last file the helper reported reading (its `"file"` events): when it dies on the spot, the + /// app names it for the next helper (`DIGUP_STOPPED_FILE`), because the queue's head is not + /// necessarily the file it died on — image batches keep all eight files pending till the batch + /// ends, and the head-marking fallback would fail an innocent one (pre-push review of #182). + private var lastReadFile: String? + private var crashFile: String? // the name to pass on, taken by the next spawn /// The next helper drops files under folders that are no longer chosen. At launch too, in case the folders /// changed while a helper had no time to; but an app with no folders at all never empties the index by itself. private var pruneWanted: Bool @@ -518,7 +536,12 @@ nonisolated final class Engine: @unchecked Sendable { arguments += extra.split(separator: " ").map(String.init) } process.arguments = arguments + roots.map(\.path) - process.environment = ProcessInfo.processInfo.environment.merging(["DIGUP_MODELS": modelsDirectory.path]) { $1 } + var environment = ["DIGUP_MODELS": modelsDirectory.path] + if let file = crashFile { // taken once: the next helper marks the file its predecessor died reading + environment["DIGUP_STOPPED_FILE"] = file + crashFile = nil + } + process.environment = ProcessInfo.processInfo.environment.merging(environment) { $1 } process.qualityOfService = .utility let input = Pipe(), output = Pipe() process.standardInput = input @@ -610,6 +633,7 @@ nonisolated final class Engine: @unchecked Sendable { + "\(int("removed")) removed · \(int("unchanged")) unchanged in \(int("ms")) ms") case "file": let (done, total) = (int("done"), int("total")) + lastReadFile = event["path"] as? String update { $0.activity = .indexing(done: done, total: total) } if event["new"] as? Bool == true, let path = event["path"] as? String { log(String(format: "index: %@ %@ in %.2f s", (path as NSString).lastPathComponent, @@ -675,6 +699,7 @@ nonisolated final class Engine: @unchecked Sendable { log("code sync: \(int("added")) new · \(int("changed")) changed · \(int("moved")) moved · " + "\(int("removed")) removed · \(int("unchanged")) unchanged in \(int("ms")) ms") case "file": + lastReadFile = event["path"] as? String update { $0.code.indexing = .init(done: int("done"), total: int("total")) } refreshCodeSearcher(force: false) case "summary": @@ -708,6 +733,7 @@ nonisolated final class Engine: @unchecked Sendable { private func applyCounts(_ event: [String: Any]) { if let files = event["files"] as? Int { + lastPending = event["pending"] as? Int update { $0.files = files $0.searchable = event["searchable"] as? Int ?? 0 @@ -746,10 +772,24 @@ nonisolated final class Engine: @unchecked Sendable { restartHelper = false startWanted = false startHelper() - } else if status == Self.stoppedOnFile { - // Its watchdog ended it on a file whose reading ran away: the next one skips that file and goes on. Not - // over and over, though, if something else is wrong. + } else if status == Self.stoppedOnFile || Self.diedOnFile(status) { + // Its watchdog ended it on a file whose reading ran away, or it died on the spot (Swift's + // allocation trap — "exited (5)" — which beats the watchdog: `CrashNotes` noted the queue's head, + // and the next helper marks it). Not over and over, though, if something else is wrong — but a + // queue that moves 100 files between stops is getting past its bad files, not looping (#182: + // a photo index of 55,000 froze on this cap after four oversized RAWs). + if Self.diedOnFile(status) { + log("indexer: it died (exit \(status)) on a file: it will be marked, and indexing goes on") + // Name it for the helper that starts next (its `CrashNotes` note carries no path): + // the queue's head-mark is a fallback, not the answer — a media batch keeps all its + // files pending, and the head of a queue of images is rarely the one being read. + crashFile = lastReadFile + } watchdogStops = watchdogStops.filter { Date().timeIntervalSince($0) < 600 } + [Date()] + if let pending = lastPending, let before = pendingAtLastStop, before - pending >= 100 { + watchdogStops = [] + } + pendingAtLastStop = lastPending if watchdogStops.count <= 3 { startHelper() } else { log("indexer: stopped on files 4 times in 10 min") } } else if syncWantedAt != nil, status == 0 { // A sync asked for while it was on its way out still has to happen. diff --git a/Sources/DigUpKit/Extractors.swift b/Sources/DigUpKit/Extractors.swift index 425407f0..be8c8bff 100644 --- a/Sources/DigUpKit/Extractors.swift +++ b/Sources/DigUpKit/Extractors.swift @@ -20,14 +20,124 @@ enum ExtractError: Error, CustomStringConvertible { } /// Pictures for the model: a downscaled copy of each image (`prepare`) and the camera's date. ImageIO on macOS (HEIC, -/// RAW, WebP and friends); stb_image elsewhere (Platform/). -enum ImageExtractor { +/// RAW, WebP and friends); stb_image elsewhere (Platform/). Public for the indexing helper to set its +/// `decodeMemoryBudget` at startup; `prepare` itself stays internal to the kit. +public enum ImageExtractor { struct Prepared: Sendable { let path: String let width: Int let height: Int let taken: String? // EXIF DateTimeOriginal, as written by the camera } + + /// What reading one picture may cost the helper in memory, set at its start from `--memory-limit` + /// (`main.swift`: the watchdog's own limit). A picture whose header says its reading would ask for more + /// goes into the index as unreadable *without being decoded*: a 45 MP RAW has run to over 8 GB in the + /// wild, killing the helper before the watchdog's tick could note the file, and every restart took that + /// file first and died on it again (issue #182: 55,000 files stuck behind it). 0 = no preflight, for the + /// tools that don't index under a watchdog. + nonisolated(unsafe) public static var decodeMemoryBudget: UInt64 = 0 + + /// Bytes the reading of one RAW pixel can take. Measured on 2026-11-05 on an Apple M1 with 8 GB RAM + /// (macOS 26.5.1): the production thumbnail path on a real 45.4 MP Nikon Z9 NEF peaks at 782 MB, 17 B/px, + /// so ImageIO of that macOS decodes RAW safely. The #182 reports — the same camera — are files whose + /// reading ran past an 8 GB watchdog, which floors the coefficient at 176 B/px; 192 carries the guard. + /// RAW is only read on the ImageIO side. Measured on one camera on one macOS, and taken for all makers. + public static let rawDecodeBytesPerPixel = 192 + + /// Bytes per pixel of a layered TIFF: every full-resolution layer decoded with its masks. A design + /// estimate, not measured — Photoshop's layered files of the #182 reports can't be synthesized from + /// nothing. Only files that say so themselves get it: `LayeredTIFF`. + static let layeredTIFFBytesPerPixel = 16 + + /// Bytes without ImageIO: stb decodes the whole picture, `oriented` copies it, and the scaling copies it + /// again (3 B/px decoded, a 4-byte copy, the scaled copy). Estimated, not measured on Linux or Windows. + static let portableDecodeBytesPerPixel = 8 + + /// A size for the file's note: "4.1 GB", "900 MB". + static func decodeSize(_ bytes: UInt64) -> String { + bytes >= 1 << 30 ? String(format: "%.1f GB", Double(bytes) / Double(1 << 30)) : "\(bytes >> 20) MB" + } +} + +/// The pixels of the layers of a Photoshop-style layered TIFF, nil when the file isn't layered. A layered +/// TIFF's master declares itself a reduced preview (`SubfileType & 1`) and keeps its full-size layers — the +/// colours, and alpha masks the same size — in sub-IFDs. A plain TIFF, including a page chain where every +/// IFD is full size, is not layered: ImageIO decodes those in streaming tiles, measured 2026-11-05 at a +/// 70 MB peak on a 2,125 MP file, so it gets no preflight. Only the headers are read: 8 bytes, 12 per +/// entry, 4 per sub-IFD — never pixels, of a file that may be 2 GB. +enum LayeredTIFF { + /// A side above this is not a picture: 16,777,216 px on a side is 280 GB of pixels at the preflight's + /// 16 B/px, and no file of it exists. The sizes come from 4-byte LONG tags, so a header saying + /// 0xFFFFFFFF is garbage — and two of those multiplied overflowed the sum below, trapping *inside* the + /// guard built to stop crashes (issue #182, pre-push review S4). Garbage gets no estimate: nil, meaning + /// "not this file's shape", so nothing is marked on it. + static let maxDimension = 1 << 24 + + /// All pixels of master and layers, counted in `UInt64` for the same reason — and saturating, so a file + /// that lies about its size produces a huge estimate, not a trap. `maxLayers`: more sub-IFDs than this + /// is not Photoshop (which caps at 50) — a file pulling at the parser is left unguarded, and the + /// watchdog ends the helper on it. + static func layerPixels(in url: URL, maxLayers: Int = 64) -> UInt64? { + guard let file = try? FileHandle(forReadingFrom: url) else { return nil } + defer { try? file.close() } + func chunk(_ at: Int, _ count: Int) -> Data? { + guard at >= 0, count > 0, count <= 1 << 20 else { return nil } + do { + try file.seek(toOffset: UInt64(at)) + guard let data = try file.read(upToCount: count), data.count == count else { return nil } + return data + } catch { + return nil // past the end, a closed file: the caller reads it as "not this file's shape" + } + } + guard let header = chunk(0, 8), header[header.startIndex] == 0x49 || header[header.startIndex] == 0x4D + else { return nil } + let big = header[header.startIndex] == 0x4D // "MM": Motorola (big-endian) order + func number(_ at: Int, _ width: Int) -> Int? { + guard let data = chunk(at, width) else { return nil } + let bytes = big ? [UInt8](data) : [UInt8](data).reversed() + return bytes.reduce(0) { $0 << 8 | Int($1) } + } + func ifd(_ at: Int) -> (subfileType: Int, width: Int, height: Int, subIFDs: [Int])? { + guard let entries = number(at, 2), entries >= 1, entries <= 1024 else { return nil } + var read = (0, 0, 0, [Int]()) + for i in 0..= 0, count <= 1 << 16 else { return nil } + let unit = type == 3 || type == 8 ? 2 : type == 1 || type == 2 || type == 6 || type == 7 ? 1 : 4 + let total = count &* unit + func value(_ index: Int) -> Int? { + if total <= 4 { return index == 0 ? number(e + 8, unit) : nil } + guard let base = number(e + 8, 4) else { return nil } + return number(base + index &* unit, unit) + } + switch tag { + case 0x00FE: read.0 = value(0) ?? 0 + case 0x0100: read.1 = value(0) ?? 0 + case 0x0101: read.2 = value(0) ?? 0 + case 0x0145 where count >= 1 && count <= maxLayers: + read.3 = total <= 4 && count == 1 ? [number(e + 8, 4) ?? 0] : (0..= 8, + let master = ifd(first), master.subfileType & 1 == 1, // the master says "I am a preview" + !master.subIFDs.isEmpty, master.width > 0, master.height > 0, + master.width <= maxDimension, master.height <= maxDimension // a header lying about its size + else { return nil } + var total = UInt64(master.width) &* UInt64(master.height) // the preview decodes too + for at in master.subIFDs { + guard at >= 8, let layer = ifd(at), layer.subfileType & 1 == 0, // a full-size layer, not a preview + layer.width > 0, layer.height > 0, + layer.width <= maxDimension, layer.height <= maxDimension else { continue } + total = total &+ UInt64(layer.width) &* UInt64(layer.height) + } + return total + } } /// Text recognition in screenshots, for keyword search only: Apple Vision on macOS, nothing elsewhere yet. diff --git a/Sources/DigUpKit/Model.swift b/Sources/DigUpKit/Model.swift index 72cb6726..7b7a4c3e 100644 --- a/Sources/DigUpKit/Model.swift +++ b/Sources/DigUpKit/Model.swift @@ -111,3 +111,40 @@ public enum CodeQuery { return trimmed.dropFirst(prefix.count).trimmingCharacters(in: .whitespacesAndNewlines) } } + +/// The file an indexing helper had to stop on (`Watchdog` in the helper, `CrashNotes` for a crash), written beside +/// the index for the next helper, which marks it as unreadable and goes on. A note with no path says "the helper +/// died on the file it was reading", and the app that watched it names that file for the next helper +/// (`DIGUP_STOPPED_FILE`, from the `file` events both apps report): the queue's head is *not* necessarily it — +/// media runs in batches of eight and the whole batch stays pending till it ends (`Indexer`), so marking the +/// head of an image queue would fail an innocent file and leave the poison queued (issue #182, pre-push +/// review S3). The head-mark is the fallback for a spawn that carries no name — a helper started from a +/// terminal, or a crash before the app saw any file. +public struct StoppedFile: Codable { + /// The exit status that says "a file was stopped: start a new helper" (EX_TEMPFAIL). + public static let exitCode: Int32 = 75 + + public let path: String? + public let why: String + + public init(path: String?, why: String) { + self.path = path + self.why = why + } + + public static func location(in folder: URL) -> URL { + folder.appendingPathComponent("stopped-file.json") + } + + public func write(to url: URL) { + try? JSONEncoder().encode(self).writeReplacing(url) + } + + /// The note an earlier helper left, removed as it's read. + public static func take(from folder: URL) -> StoppedFile? { + let url = location(in: folder) + guard let data = try? Data(contentsOf: url) else { return nil } + try? FileManager.default.removeItem(at: url) + return try? JSONDecoder().decode(StoppedFile.self, from: data) + } +} diff --git a/Sources/DigUpKit/Platform/Apple/Extractors+Apple.swift b/Sources/DigUpKit/Platform/Apple/Extractors+Apple.swift index 36a68dba..4d52c158 100644 --- a/Sources/DigUpKit/Platform/Apple/Extractors+Apple.swift +++ b/Sources/DigUpKit/Platform/Apple/Extractors+Apple.swift @@ -23,6 +23,40 @@ extension ImageExtractor { let width = properties[kCGImagePropertyPixelWidth] as? Int ?? 0 let height = properties[kCGImagePropertyPixelHeight] as? Int ?? 0 guard max(width, height) >= minSide else { throw ExtractError.tooSmall(width, height) } + // The preflight of issue #182, for the two kinds of picture that ran away in the field: RAW, and + // TIFFs whose master says its full-resolution layers live in sub-IFDs. The reading of both has + // ballooned past gigabytes on real photos (a 45 MP Nikon RAW ran past an 8 GB watchdog), and the + // helper died on them before the watchdog tick could note the file. Both are judged from the + // header, which ImageIO has already read for us: pixels, and the master's own declaration. + // Plain TIFFs, JPEGs, PNGs get nothing: their scaled decode streams in tiles, measured at 70 MB + // for a 2,125 MP TIFF and 18 MB for a 100 MP PNG (2026-11-05, Apple M1 with 8 GB RAM). + let budget = decodeMemoryBudget + if budget > 0 { + let uti = (CGImageSourceGetType(source) as String?).flatMap { UTType($0) } + // Pixels and their estimates in `UInt64`, saturating: ImageIO reports the header's own numbers, + // and a file lying about them (a 4-byte dimension of 0xFFFFFFFF, a negative one) must give a + // huge estimate, not an overflow trap in the guard built to stop traps (#182 review S4). The + // budget is a `UInt64` too: comparing in `Int` needed `Int(budget)`, which itself traps above + // `Int.max` on an absurd `--memory-limit` (review N1). + let pixels = UInt64(max(width, 0)) &* UInt64(max(height, 0)) + // Every camera RAW UTI (com.nikon.raw-image and friends) conforms to public.raw-image. + if uti?.conforms(to: .rawImage) == true { + let estimate = pixels &* UInt64(rawDecodeBytesPerPixel) + guard estimate <= budget else { + throw ExtractError.unreadable("needed an estimated \(decodeSize(estimate)) of memory to read: " + + "a RAW picture of \(pixels / 1_000_000) MP " + + "(the helper's limit is \(decodeSize(budget)))") + } + } else if uti?.conforms(to: .tiff) == true, let layers = LayeredTIFF.layerPixels(in: url) { + let estimate = (pixels &+ layers) &* UInt64(layeredTIFFBytesPerPixel) + guard estimate <= budget else { + throw ExtractError.unreadable("needed an estimated \(decodeSize(estimate)) of memory to read: " + + "a layered TIFF, \(pixels / 1_000_000) MP of preview and " + + "\(layers / 1_000_000) MP of layers (the helper's limit is " + + "\(decodeSize(budget)))") + } + } + } let options: [CFString: Any] = [ kCGImageSourceCreateThumbnailFromImageAlways: true, kCGImageSourceThumbnailMaxPixelSize: maxPixel, diff --git a/Sources/DigUpKit/Platform/Portable/Images+Portable.swift b/Sources/DigUpKit/Platform/Portable/Images+Portable.swift index 97a703b5..5b7c861f 100644 --- a/Sources/DigUpKit/Platform/Portable/Images+Portable.swift +++ b/Sources/DigUpKit/Platform/Portable/Images+Portable.swift @@ -19,6 +19,20 @@ extension ImageExtractor { if stbi_info(url.path, &storedWidth, &storedHeight, &channels) != 0 { (width, height) = (Int(storedWidth), Int(storedHeight)) guard max(width, height) >= minSide else { throw ExtractError.tooSmall(width, height) } + // Here the whole picture is decoded, turned, and scaled (`decodeWithFFmpeg` pipes the same), so + // full-size pixels are the peak and the header alone can tell it: a giant JPEG decodes even + // where ImageIO would stream tiles (issue #182's preflight, on the stb side). `UInt64` and + // saturating: stb reports the header's own numbers, and a file lying about them (a 4-byte + // dimension of 0xFFFFFFFF) must give a huge estimate, not an overflow trap in the guard built + // to stop traps (#182 review S4); comparing in `UInt64` also avoids `Int(budget)`, which traps + // above `Int.max` on an absurd `--memory-limit` (review N1). + let pixels = UInt64(max(width, 0)) &* UInt64(max(height, 0)) + let estimate = pixels &* UInt64(portableDecodeBytesPerPixel) + guard decodeMemoryBudget == 0 || estimate <= decodeMemoryBudget else { + throw ExtractError.unreadable("needed an estimated \(decodeSize(estimate)) of memory to read: " + + "a picture of \(pixels / 1_000_000) MP " + + "(the helper's limit is \(decodeSize(decodeMemoryBudget)))") + } guard let decoded = RGBImage.load(url.path) else { throw ExtractError.unreadable("can't decode image") } image = decoded.oriented(exif?.orientation ?? 1) } else { @@ -51,10 +65,22 @@ extension ImageExtractor { throw ExtractError.unreadable("ffmpeg isn't installed (HEIC, WebP and TIFF pictures need it)") } var data = Data() + let budget = decodeMemoryBudget do { try FFmpeg.run(FFmpeg.ffmpeg, ["-v", "error", "-nostdin", "-i", url.path, "-frames:v", "1", "-pix_fmt", - "rgb24", "-f", "image2pipe", "-c:v", "ppm", "-"]) { data.append($0) } + "rgb24", "-f", "image2pipe", "-c:v", "ppm", "-"]) { chunk in + data.append(chunk) + // The picture whole, made as PPM over the pipe (3 bytes per pixel, before any scaling), + // lands in the helper's own memory: the helper's limit stops the reading, and `FFmpeg.run` + // stops ffmpeg with it (issue #182: this is where a huge TIFF/HEIC lands on this side). + // Compared in `UInt64`: `Int(budget)` traps above `Int.max`, which an absurd + // `--memory-limit` can hand us (#182 review N1). + guard budget == 0 || UInt64(data.count) <= budget else { + throw ExtractError.unreadable("needed more than \(decodeSize(budget)) of memory to read") + } + } } catch { + if let error = error as? ExtractError { throw error } // the memory cap above, not a broken picture let ext = url.pathExtension.lowercased() if ext == "heic" || ext == "heif" { throw ExtractError.unreadable("this ffmpeg can't read HEIC (8.1 can)") } throw ExtractError.unreadable("can't decode image") diff --git a/Sources/DigUpKit/Platform/Portable/Media+Portable.swift b/Sources/DigUpKit/Platform/Portable/Media+Portable.swift index ee47f71c..5910d7cd 100644 --- a/Sources/DigUpKit/Platform/Portable/Media+Portable.swift +++ b/Sources/DigUpKit/Platform/Portable/Media+Portable.swift @@ -35,7 +35,16 @@ enum FFmpeg { while true { let data = reader.readData(ofLength: 1 << 16) if data.isEmpty { break } - try read(data) + do { + try read(data) + } catch { + // The reader has had enough (a picture that would overrun the helper's memory, the cap in + // `decodeWithFFmpeg`): ending ffmpeg here, not leaving it to block on a full pipe forever + // with the helper gone past it. + process.terminate() + process.waitUntilExit() + throw error + } } process.waitUntilExit() guard process.terminationStatus == 0 else { diff --git a/Sources/digup/CrashNotes.swift b/Sources/digup/CrashNotes.swift new file mode 100644 index 00000000..a658b37e --- /dev/null +++ b/Sources/digup/CrashNotes.swift @@ -0,0 +1,69 @@ +import DigUpKit +import Foundation +#if canImport(Darwin) +import Darwin +#else +import Glibc +#endif + +/// Notes the file the helper dies on when a fatal signal beats the watchdog. Swift's allocation trap — the +/// "indexer: exited (5)" of issue #182 — fires on the first allocation the system refuses, inside a hundredth +/// of a second of a 45 MP RAW's reading, faster than a watchdog tick could note the file. The file then stays +/// pending, every restart takes it first, and the queue never moves (55,000 files behind it in the #182 +/// report). These handlers write a `StoppedFile` note and re-raise, so the next helper marks that file and +/// indexing goes on. The note carries no path (the crash beats `Indexer.nowReading`, and reading runs ahead of +/// the queue's head in media batches): the app that watched the helper names the file for the next one +/// (`DIGUP_STOPPED_FILE`, from the `file` events), and the head-mark is the fallback. +/// The handler runs off install-time copies with syscalls alone (`open`, `write`): malloc and Foundation +/// can deadlock a process that was mid-allocation when the signal arrived, and the helper's startup consumes +/// any note left beside the index, so the file is made at the crash itself, by path. +/// Installing plain handlers replaces Swift's own signal handling: Swift's crash print ("Fatal machine fault" +/// with a backtrace, its "Incident Identifier" report on macOS) is lost, deliberately — the note beside the +/// index is what unblocks a queue, and a backtrace in the log is not. Re-raising at `SIG_DFL` keeps what the +/// system can build from the signal itself (macOS writes its `.ips` report on the re-raise; on Linux Swift's +/// print is gone, which is the trade). Chaining Swift's old handler via `sigaction` would keep both, and is +/// the natural next step here. On Windows the C runtime translates only SEGV, FPE and ILL — a Swift trap +/// arrives as an SEH exit code, not one of these signals, so the note may be absent there; the shell then +/// marks nothing, and the restart cap bounds the loop (disclosed in issue #182's notes). +enum CrashNotes { + nonisolated(unsafe) static var notePath: UnsafeMutablePointer? + nonisolated(unsafe) static var noteBytes: UnsafeMutableRawPointer? + nonisolated(unsafe) static var noteLength: Int = 0 + + static func install(in folder: URL) { + let note = StoppedFile(path: nil, why: "crashed on it (a fatal signal — exit 5 among them) while reading it") + guard let data = try? JSONEncoder().encode(note) else { return } + let bytes = UnsafeMutableRawPointer.allocate(byteCount: max(data.count, 1), alignment: 1) + data.withUnsafeBytes { bytes.copyMemory(from: $0.baseAddress!, byteCount: data.count) } + guard let path = strdup(StoppedFile.location(in: folder).path) else { + bytes.deallocate() + return + } + notePath = path + noteBytes = bytes + noteLength = data.count + // SIGTRAP is Swift's trap; SIGSEGV/SIGBUS the access faults; SIGABRT/SIGILL/SIGFPE the rest. + // The Windows C runtime has no SIGBUS; its faults all arrive as SIGSEGV. + #if os(Windows) + let fatal: [Int32] = [SIGABRT, SIGFPE, SIGILL, SIGSEGV, SIGTRAP] + #else + let fatal: [Int32] = [SIGABRT, SIGBUS, SIGFPE, SIGILL, SIGSEGV, SIGTRAP] + #endif + for sig in fatal { signal(sig, crashNoteAndReraise) } + } +} + +/// Runs on the crashing thread. Everything it touches was made at `install`; `open` and `write` are +/// async-signal-safe, so this notes the crash and then lets the default handler take the process: the app +/// sees the same exit it saw before, with a note beside the index for the next helper to mark the file. +func crashNoteAndReraise(_ sig: Int32) { + if let path = CrashNotes.notePath, let bytes = CrashNotes.noteBytes { + let fd = open(path, O_WRONLY | O_CREAT | O_TRUNC, 0o644) + if fd >= 0 { + _ = write(fd, bytes, CrashNotes.noteLength) + _ = close(fd) + } + } + signal(sig, SIG_DFL) + raise(sig) +} diff --git a/Sources/digup/IndexService.swift b/Sources/digup/IndexService.swift index db2a807f..5435bdd4 100644 --- a/Sources/digup/IndexService.swift +++ b/Sources/digup/IndexService.swift @@ -81,10 +81,32 @@ final class IndexService: @unchecked Sendable { removeStaleScratchFolders() do { // A file an earlier helper had to stop on (`Watchdog`) isn't read again until it changes. - if let stopped = StoppedFile.take(from: folder), let path = stopped.path { - for part in parts { - guard try part.store.markStopped(path, why: stopped.why) else { continue } - emit(["event": "stopped", "path": path, "why": stopped.why, "code": part.code]) + if let stopped = StoppedFile.take(from: folder) { + if let path = stopped.path { + for part in parts { + guard try part.store.markStopped(path, why: stopped.why) else { continue } + emit(["event": "stopped", "path": path, "why": stopped.why, "code": part.code]) + } + } else { + // A note without a path is "the helper died on the file it was reading" (`CrashNotes`). + // The app names that file for the next helper (`DIGUP_STOPPED_FILE`): the queue's head + // is *not* necessarily it — images run eight at a time and the whole batch stays + // pending till it ends, so marking the head would fail an innocent file and leave the + // poison queued (pre-push review of #182). The head-mark stays for a spawn that + // carries no name, and marks nothing when the queue has moved on. + let named = ProcessInfo.processInfo.environment["DIGUP_STOPPED_FILE"] + var marked = false + if let named { + for part in parts { + guard try part.store.markStopped(named, why: stopped.why) else { continue } + emit(["event": "stopped", "path": named, "why": stopped.why, "code": part.code]) + marked = true + } + } + if !marked, let head = try main.store.pending(limit: 1).first { + try main.store.markStopped(head.path, why: stopped.why) + emit(["event": "stopped", "path": head.path, "why": stopped.why]) + } } } let again = try main.store.readAgainOnce() // what older indexes couldn't read: scans' words, SVGs diff --git a/Sources/digup/Watchdog.swift b/Sources/digup/Watchdog.swift index 5d45abe0..a0a11b1b 100644 --- a/Sources/digup/Watchdog.swift +++ b/Sources/digup/Watchdog.swift @@ -3,10 +3,12 @@ import Foundation /// Ends the indexing helper when reading one file runs away. Apple's PDF text extraction has grown without end on a /// page of some PDFs (30 GB in 3.5 minutes, never returning), and nothing stops code like that from outside but ending -/// the process. Every second the watchdog looks at the helper's memory (its footprint, as Activity Monitor counts it) and -/// at how long the file being read (`Indexer.nowReading`) has taken. Past `memoryLimit`, or past `timeLimit` on one -/// file, it writes down the file and why (`StoppedFile`) and exits with `StoppedFile.exitCode`: the app starts a new -/// helper, which marks that file as one that couldn't be read and goes on. +/// the process. Ten times a second the watchdog looks at the helper's memory (its footprint, as Activity Monitor +/// counts it) and at how long the file being read (`Indexer.nowReading`) has taken. Past `memoryLimit`, or past +/// `timeLimit` on one file, it writes down the file and why (`StoppedFile`) and exits with `StoppedFile.exitCode`: +/// the app starts a new helper, which marks that file as one that couldn't be read and goes on. The ten-times-a-second +/// rate matters: the runaway reading of a 45 MP RAW climbs past gigabytes in well under a second, and a watchdog +/// that looks once a second loses it to Swift's allocation trap, which notes nothing (issue #182). final class Watchdog: @unchecked Sendable { let memoryLimit: UInt64 let timeLimit: TimeInterval @@ -30,7 +32,7 @@ final class Watchdog: @unchecked Sendable { func start() { Thread.detachNewThread { [self] in while true { - Thread.sleep(forTimeInterval: 1) + Thread.sleep(forTimeInterval: 0.1) let reading = Indexer.nowReading if let footprint = Self.footprint(), footprint > memoryLimit { stop(reading?.path, why: "needed more than \(Self.size(memoryLimit)) of memory to read") @@ -78,28 +80,3 @@ final class Watchdog: @unchecked Sendable { #endif } } - -/// The file a helper stopped on (`Watchdog`), written beside the index for the next helper. -struct StoppedFile: Codable { - /// The exit status that says "a file was stopped: start a new helper" (EX_TEMPFAIL). - static let exitCode: Int32 = 75 - - let path: String? - let why: String - - static func location(in folder: URL) -> URL { - folder.appendingPathComponent("stopped-file.json") - } - - func write(to url: URL) { - try? JSONEncoder().encode(self).writeReplacing(url) - } - - /// The note an earlier helper left, removed as it's read. - static func take(from folder: URL) -> StoppedFile? { - let url = location(in: folder) - guard let data = try? Data(contentsOf: url) else { return nil } - try? FileManager.default.removeItem(at: url) - return try? JSONDecoder().decode(StoppedFile.self, from: data) - } -} diff --git a/Sources/digup/main.swift b/Sources/digup/main.swift index 38c3ba2c..54ae78bf 100644 --- a/Sources/digup/main.swift +++ b/Sources/digup/main.swift @@ -437,10 +437,17 @@ func indexService(_ args: Arguments) throws -> Int32 { codeOptions.code = true codeOptions.excludedFolders = (args.lists["--code-exclude"] ?? []).map { Arguments.folderURL($0).path } let codeRoots = (args.lists["--code-root"] ?? []).map(Arguments.folderURL) - // A file whose reading runs away ends this helper, noted for the next one (`Watchdog`). - Watchdog(folder: args.indexFolder, - memoryLimit: args.values["--memory-limit"].flatMap(UInt64.init).map { $0 << 20 } ?? Watchdog.defaultMemoryLimit, + // A file whose reading runs away ends this helper, noted for the next one (`Watchdog`). `&*` and not + // `<<`: a `--memory-limit` above 2^44 MB would overflow-shift and trap in the helper's own setup + // (#182 review N1); saturating gives a limit no file reaches, which is what such an argument means. + let memoryLimit = args.values["--memory-limit"].flatMap(UInt64.init).map { $0 &* (1 << 20) } ?? Watchdog.defaultMemoryLimit + Watchdog(folder: args.indexFolder, memoryLimit: memoryLimit, timeLimit: args.values["--time-limit"].flatMap(Double.init) ?? Watchdog.defaultTimeLimit).start() + // A picture whose header says its reading would ask for more than that is marked, not decoded, and a + // helper killed on the spot (Swift's allocation trap) is noted too: a 45 MP RAW can hold the whole + // queue at its head (issue #182; the report's 55,000 files waited behind one). + ImageExtractor.decodeMemoryBudget = memoryLimit + CrashNotes.install(in: args.indexFolder) let service = IndexService(main: try part(IndexStore.mainFile, options: args.options, roots: args.positional.map(Arguments.folderURL)), code: codeRoots.isEmpty ? nil : try part(IndexStore.codeFile, options: codeOptions, diff --git a/Tests/DigUpKitTests/PreflightTests.swift b/Tests/DigUpKitTests/PreflightTests.swift new file mode 100644 index 00000000..a0eaaaef --- /dev/null +++ b/Tests/DigUpKitTests/PreflightTests.swift @@ -0,0 +1,251 @@ +import Foundation +import Testing +@testable import DigUpKit + +/// Issue #182: the preflight that marks a picture from its header before the helper decodes it, the +/// layered-TIFF parser, the stopped-file note the helper leaves when it dies on a file, and the queue head +/// that note marks. The synthetic files are built from bytes here, so they run on CI (no fixtures, no +/// network); the tests that need a real RAW camera file take it from `DIGUP_TEST_RAW` and stay disabled +/// without it (measured on the box that ran the #182 calibration: Apple M1, 8 GB RAM, macOS 26.5.1, a +/// real 45.44 MP Nikon Z9 NEF). +@Suite(.serialized) struct PreflightTests { + let folder: URL = { + let url = FileManager.default.temporaryDirectory.appendingPathComponent("PreflightTests-\(UUID().uuidString)") + try! FileManager.default.createDirectory(at: url, withIntermediateDirectories: true) + return url + }() + + // MARK: The layered-TIFF parser + + func u16(_ v: Int) -> [UInt8] { [UInt8(v & 0xFF), UInt8((v >> 8) & 0xFF)] } + func u32(_ v: Int) -> [UInt8] { [UInt8(v & 0xFF), UInt8((v >> 8) & 0xFF), UInt8((v >> 16) & 0xFF), UInt8((v >> 24) & 0xFF)] } + func sh(_ v: Int) -> [UInt8] { u16(v) } + + func entry(_ tag: Int, _ type: Int, _ count: Int, _ value: [UInt8]) -> [UInt8] { + var out = u16(tag) + u16(type) + u32(count) + value + while out.count < 12 { out.append(0) } + return out + } + + func ifd(_ entries: [(tag: Int, type: Int, count: Int, value: [UInt8])]) -> [UInt8] { + var out = u16(entries.count) + for e in entries { out += entry(e.tag, e.type, e.count, e.value) } + out += u32(0) // no next IFD + return out + } + + var short8: [UInt8] { u16(8) } // BitsPerSample 8 as a stored SHORT + + /// A Photoshop-style layered TIFF: a master that declares itself a preview (`SubfileType & 1`) and + /// keeps its full-size layers in sub-IFDs. Sizes: master 800×600, layers 3000×3000, 3000×3000, 1500×1500. + func layeredTIFF(layers: [(Int, Int)] = [(3000, 3000), (3000, 3000), (1500, 1500)], + master: (width: Int, height: Int) = (800, 600)) -> Data { + let n = layers.count + let offsetsAt = 8 + (2 + 10 * 12 + 4) + let layersAt = offsetsAt + 4 * n + let dataAt = layersAt + n * (2 + 3 * 12 + 4) + var bytes = [UInt8]("II".utf8) + u16(42) + u32(8) + bytes += ifd([ + (0x00FE, 4, 1, u32(1)), + (0x0100, 4, 1, u32(master.width)), (0x0101, 4, 1, u32(master.height)), + (0x0102, 3, 1, short8), (0x0103, 3, 1, sh(1)), (0x0106, 3, 1, sh(1)), + (0x0111, 4, 1, u32(dataAt)), (0x0115, 3, 1, sh(1)), (0x0116, 3, 1, sh(600)), + (0x0145, 4, n, u32(offsetsAt)), + ]) + for i in 0.. Data { + var bytes = [UInt8]("II".utf8) + u16(42) + u32(8) + bytes += ifd([ + (0x00FE, 4, 1, u32(0)), + (0x0100, 4, 1, u32(side)), (0x0101, 4, 1, u32(side)), + (0x0102, 3, 1, short8), (0x0103, 3, 1, sh(1)), (0x0106, 3, 1, sh(1)), + (0x0111, 4, 1, u32(8 + 2 + 9 * 12 + 4)), (0x0115, 3, 1, sh(1)), (0x0116, 3, 1, sh(side)), + ]) + bytes += [0] + return Data(bytes) + } + + func write(_ data: Data, _ name: String) -> URL { + let url = folder.appendingPathComponent(name) + try! data.write(to: url) + return url + } + + @Test func parserSumsTheLayersOfALayeredTIFF() throws { + let file = write(layeredTIFF(), "layered.tif") + let pixels: UInt64 = 800 * 600 + 3000 * 3000 + 3000 * 3000 + 1500 * 1500 + #expect(LayeredTIFF.layerPixels(in: file) == pixels) + } + + @Test func parserLeavesAPlainTIFFAlone() throws { + #expect(LayeredTIFF.layerPixels(in: write(plainTIFF(side: 3000), "plain.tif")) == nil) + } + + @Test func parserLeavesFilesPastItsLayerLimitAlone() throws { + // More sub-IFDs than the parser signs up for is not Photoshop: leave it unguarded (the watchdog + // ends the helper on it) rather than guess at a shape the parser is reading wrong. + let bytes = layeredTIFF(layers: (0..<70).map { _ in (3000, 3000) }) + #expect(LayeredTIFF.layerPixels(in: write(bytes, "many.tif")) == nil) + #expect(LayeredTIFF.layerPixels(in: write(layeredTIFF(layers: (0..<64).map { _ in (100, 100) }), "fit.tif")) != nil) + } + + @Test func parserRefusesGarbageDimensions() throws { + // The sizes are 4-byte LONG tags, so a header can say 0xFFFFFFFF: garbage, and the product of two + // such sides overflowed the parser's `Int` sum — a trap *inside* the guard built to stop traps + // (#182, pre-push review S4). Garbage gets no estimate at all (nil: "not this file's shape"), so + // nothing is marked from it, and a lying layer is dropped from a sum that is otherwise sound. + let garbageMaster = write(layeredTIFF(layers: [(100, 100)], master: (width: 0xFFFFFFFF, height: 0xFFFFFFFF)), + "garbage-master.tif") + #expect(LayeredTIFF.layerPixels(in: garbageMaster) == nil) + let garbageLayer = write(layeredTIFF(layers: [(0xFFFFFFFF, 0xFFFFFFFF), (3000, 3000)]), "garbage-layer.tif") + let sound: UInt64 = 800 * 600 + 3000 * 3000 + #expect(LayeredTIFF.layerPixels(in: garbageLayer) == sound) + // The sum itself is counted in `UInt64`: 64 layers of 16M×16M saturates at the ceiling, it does not trap. + let enormous = write(layeredTIFF(layers: (0..<64).map { _ in (1 << 24, 1 << 24) }), "enormous.tif") + #expect(LayeredTIFF.layerPixels(in: enormous) != nil) + } + + @Test func sizesReadAsWords() { #expect(ImageExtractor.decodeSize(192 << 20) == "192 MB") + #expect(ImageExtractor.decodeSize(8 << 30) == "8.0 GB") + } + + // MARK: The preflight through the production path (macOS: ImageIO side) + + #if canImport(ImageIO) + @Test func layeredTIFFOverBudgetIsMarkedBeforeDecoding() throws { + let file = write(layeredTIFF(), "marked.tif") + ImageExtractor.decodeMemoryBudget = 300 << 20 // the estimate is 800×600+20.25 MP at 16 B = 323 MB + defer { ImageExtractor.decodeMemoryBudget = 0 } + do { + _ = try ImageExtractor.prepare(file, maxPixel: 1024, minSide: 256, to: folder, png: false) + Issue.record("the preflight let a layered TIFF over the budget through") + } catch let error as ExtractError { + guard case .unreadable(let why) = error, why.contains("estimated") else { + Issue.record("expected the preflight's mark, got \(error)") + return + } + } + } + + @Test func layeredTIFFUnderBudgetGoesThrough() throws { + let file = write(layeredTIFF(), "under.tif") + ImageExtractor.decodeMemoryBudget = 4096 << 20 // 323 MB is well under 4 GB + defer { ImageExtractor.decodeMemoryBudget = 0 } + _ = try ImageExtractor.prepare(file, maxPixel: 1024, minSide: 256, to: folder, png: false) + } + + @Test func budgetOffMeansNoPreflight() throws { + let file = write(layeredTIFF(), "off.tif") + ImageExtractor.decodeMemoryBudget = 0 + _ = try ImageExtractor.prepare(file, maxPixel: 1024, minSide: 256, to: folder, png: false) + } + + // Real-RAW tests: a camera file on the box running the suite (DIGUP_TEST_RAW=… .nef). The #182 + // calibration: this Z9 NEF decodes at a 782 MB peak on this macOS (17 B/px) and the guard fires at + // 192 B/px — marked under a 4 GB helper limit, decoded with a 16 GB one. + static let rawFile: URL? = ProcessInfo.processInfo.environment["DIGUP_TEST_RAW"].map(URL.init(fileURLWithPath:)) + + @Test(.enabled(if: rawFile != nil)) func realRAWOverTheHelperLimitIsMarked() throws { + guard let raw = Self.rawFile else { return } + ImageExtractor.decodeMemoryBudget = 4096 << 20 + defer { ImageExtractor.decodeMemoryBudget = 0 } + #expect(throws: ExtractError.self) { + try ImageExtractor.prepare(raw, maxPixel: 1024, minSide: 256, to: folder, png: false) + } + } + + @Test(.enabled(if: rawFile != nil)) func realRAWWithRoomToSpareDecodes() throws { + guard let raw = Self.rawFile else { return } + ImageExtractor.decodeMemoryBudget = 16384 << 20 + defer { ImageExtractor.decodeMemoryBudget = 0 } + let prepared = try ImageExtractor.prepare(raw, maxPixel: 1024, minSide: 256, to: folder, png: false) + #expect(prepared.width * prepared.height >= 20_000_000) // a big camera file, decoded whole + } + #endif + + // MARK: The preflight on the stb side (Linux, Windows): full decode, so the header alone marks + + #if !canImport(ImageIO) + func picture(_ side: Int, _ name: String) -> URL { + let image = RGBImage(width: side, height: side, pixels: [UInt8](repeating: 128, count: side * side * 3)) + let url = folder.appendingPathComponent(name) + try! image.write(to: url, png: true) + return url + } + + @Test func pictureOverBudgetIsMarkedBeforeDecodingHere() throws { + let file = picture(800, "over.png") // 800×800 at 8 B/px = 6.4 MB + ImageExtractor.decodeMemoryBudget = 4 << 20 + defer { ImageExtractor.decodeMemoryBudget = 0 } + do { + _ = try ImageExtractor.prepare(file, maxPixel: 1024, minSide: 256, to: folder, png: true) + Issue.record("the preflight let a picture over the budget through") + } catch let error as ExtractError { + guard case .unreadable(let why) = error, why.contains("estimated") else { + Issue.record("expected the preflight's mark, got \(error)") + return + } + } + } + + @Test func pictureUnderBudgetDecodesHere() throws { + let file = picture(800, "under.png") + ImageExtractor.decodeMemoryBudget = 64 << 20 + defer { ImageExtractor.decodeMemoryBudget = 0 } + let prepared = try ImageExtractor.prepare(file, maxPixel: 1024, minSide: 256, to: folder, png: true) + #expect(prepared.width == 800 && prepared.height == 800) + } + #endif + + // MARK: The stopped-file note, with and without a path + + @Test func notesRoundTripAndAreConsumedOnce() throws { + let folder = self.folder.appendingPathComponent("notes-\(UUID().uuidString)") + try FileManager.default.createDirectory(at: folder, withIntermediateDirectories: true) + #expect(StoppedFile.take(from: folder) == nil) + StoppedFile(path: "/a/b.NEF", why: "needed more than 4 GB of memory to read").write(to: StoppedFile.location(in: folder)) + let note = try #require(StoppedFile.take(from: folder)) + #expect(note.path == "/a/b.NEF") + #expect(StoppedFile.take(from: folder) == nil) // taken: gone + StoppedFile(path: nil, why: "crashed on it").write(to: StoppedFile.location(in: folder)) + let head = try #require(StoppedFile.take(from: folder)) + #expect(head.path == nil && head.why.contains("crashed")) + } + + @Test func aPathlessNoteMarksTheQueueHead() throws { + // What `IndexService` does at startup with a note that names no file: the queue's head — the + // file an index helper was reading when it died — is marked, and the next file becomes the head. + let store = try IndexStore(directory: folder) + func file(_ name: String, _ inode: UInt64) -> Candidate { + Candidate(path: folder.appendingPathComponent(name).path, kind: .image, size: 100, mtime: 1, + inode: inode, device: 1) + } + try store.insert(file("first.NEF", 1)) + try store.insert(file("second.NEF", 2)) + let head = try #require(try store.pending(limit: 1).first) + #expect(head.path.hasSuffix("first.NEF")) + try store.markStopped(head.path, why: "crashed on it (a fatal signal)") + let next = try #require(try store.pending(limit: 1).first) + #expect(next.path.hasSuffix("second.NEF")) // the queue moves: #182's stall broken + let marked = try store.files(under: [folder.path]).first { $0.path.hasSuffix("first.NEF") } + #expect(marked?.state == "failed") // "couldn't be read": the reporter's expectation + } + + @Test func aMarkedFileStaysMarkedUntilItChanges() throws { + let store = try IndexStore(directory: folder) + try store.insert(Candidate(path: folder.appendingPathComponent("a.NEF").path, kind: .image, size: 100, + mtime: 1, inode: 1, device: 1)) + #expect(try store.markStopped(folder.appendingPathComponent("a.NEF").path, why: "needed more than 4 GB")) + let pending = try store.pending(limit: nil) + #expect(pending.map(\.path).isEmpty) // not queued again + } +} diff --git a/shell/src-tauri/src/helper.rs b/shell/src-tauri/src/helper.rs index fed4d159..5708d045 100644 --- a/shell/src-tauri/src/helper.rs +++ b/shell/src-tauri/src/helper.rs @@ -99,16 +99,33 @@ fn stop(mut child: Child, mut stdin: ChildStdin, farewell: &str) { /// Each helper started gets a number, so the thread reading it knows whether its slot still holds it or a newer one. static STARTED: AtomicU64 = AtomicU64::new(1); -/// A helper whose output ended is on its way out: waits for it (up to five seconds) and gives its exit code. -fn reap(child: &mut Child) -> Option { +/// The fatal signal a child died by, where the platform reports one (`ExitStatus::signal()` is Unix-only — +/// `ExitStatusExt`, stable since 1.0 — and Windows carries no signals: its crash class arrives as the SEH +/// exit code, which `index_ended` in main.rs matches). +#[cfg(unix)] +fn fatal_signal(status: &std::process::ExitStatus) -> Option { + use std::os::unix::process::ExitStatusExt; + status.signal() +} + +#[cfg(windows)] +fn fatal_signal(_: &std::process::ExitStatus) -> Option { + None +} + +/// A helper whose output ended is on its way out: waits for it (up to five seconds) and gives its exit +/// code — and the fatal signal it died by, which is what `ExitStatus::code()` does not report on Unix +/// (a child killed by a signal has no exit code at all: issue #182's crash classification is +/// unreachable from `code()` alone, and on Windows the SEH exit codes carry the crash class instead). +fn reap(child: &mut Child) -> (Option, Option) { for _ in 0..100 { match child.try_wait() { - Ok(Some(status)) => return status.code(), + Ok(Some(status)) => return (status.code(), fatal_signal(&status)), Ok(None) => std::thread::sleep(Duration::from_millis(50)), - Err(_) => return None, + Err(_) => return (None, None), } } - None + (None, None) } /// `digup search-service`: started on the first request, and again after it died. @@ -262,8 +279,8 @@ impl IndexService { if slot.as_ref().is_some_and(|indexing| indexing.number == number) { if let Some(mut indexing) = slot.take() { drop(slot); - let status = reap(&mut indexing.child); - on_event(json!({"event": "ended", "status": status, "owed": indexing.owed})); + let (status, signal) = reap(&mut indexing.child); + on_event(json!({"event": "ended", "status": status, "signal": signal, "owed": indexing.owed})); } } }); @@ -409,3 +426,35 @@ mod job { } } } + +#[cfg(all(test, unix))] +mod reap_tests { + use super::reap; + use std::process::{Command, Stdio}; + + /// The exit a helper leaves behind, read the way the shell reads it (issue #182's crash + /// classification): a child killed by a signal has no exit code on Unix — `code()` reports nothing, + /// and the signal is what carries the class. `sh`'s own `kill` is a built-in, so `$$` is the child. + #[test] + fn a_kill_by_signal_gives_the_signal_and_no_code() { + let mut child = Command::new("/bin/sh") + .args(["-c", "kill -TRAP $$"]) + .stdout(Stdio::null()) + .stderr(Stdio::null()) + .spawn() + .unwrap(); + assert_eq!(reap(&mut child), (None, Some(5))); // SIGTRAP: Swift's allocation trap + } + + /// The other shape: a deliberate exit (the watchdog's stop is 75, EX_TEMPFAIL), with no signal. + #[test] + fn a_plain_exit_gives_its_code_and_no_signal() { + let mut child = Command::new("/bin/sh") + .args(["-c", "exit 75"]) + .stdout(Stdio::null()) + .stderr(Stdio::null()) + .spawn() + .unwrap(); + assert_eq!(reap(&mut child), (Some(75), None)); + } +} diff --git a/shell/src-tauri/src/main.rs b/shell/src-tauri/src/main.rs index 375fbee2..871710f7 100644 --- a/shell/src-tauri/src/main.rs +++ b/shell/src-tauri/src/main.rs @@ -28,7 +28,7 @@ use serde_json::{json, Value}; use settings::Settings; use status::Status; use std::path::{Path, PathBuf}; -use std::sync::atomic::{AtomicBool, AtomicUsize, Ordering}; +use std::sync::atomic::{AtomicBool, AtomicI64, AtomicUsize, Ordering}; use std::sync::{Arc, Mutex}; use std::time::{Duration, Instant}; use tauri::menu::{Menu, MenuItem, PredefinedMenuItem}; @@ -87,6 +87,16 @@ struct Shell { reloaded: Mutex, /// When the index helper's watchdog ended it lately (on a file whose reading ran away). watchdog_stops: Mutex>, + /// The queue's pending-file count at the last stop on a file (`status.pending` is the count now): stops + /// are a stall only when the queue isn't moving between them (issue #182, like the Mac app's `Engine`). + pending_at_stop: AtomicI64, + /// The last file the index helper reported reading (its `"file"` event). When a helper dies on the + /// spot, this is the app's best clue which file to mark: the queue's head is not necessarily it + /// (images run eight at a time, all eight stay pending till the batch ends — pre-push review #182). + last_file: Mutex>, + /// The file to hand the next helper as `DIGUP_STOPPED_FILE`, set when a fatal-signal exit is seen, + /// taken by the spawn that uses it. + crash_file: Mutex>, /// The window's size inside its frame, as its page last reported it (GTK's own counts its title bar and shadow). window_inner: Mutex>, /// A search the panel hands the window (its query, and the kind of results to show): taken when the window opens. @@ -749,6 +759,9 @@ fn setup(app: &mut tauri::App, options: Options) -> Result<(), Box>().status.lock().unwrap().apply(&event); match event["event"].as_str().unwrap_or_default() { - "file" | "rest" => reload(app, false), + "file" => { + app.state::>().last_file + .lock() + .unwrap() + .clone_from(&event["path"].as_str().map(str::to_string)); + reload(app, false) + } + "rest" => reload(app, false), "idle" => { reload(app, true); // A debug run through setup searches once the first index is in. @@ -1868,22 +1896,53 @@ fn index_event(app: &AppHandle, event: Value) { let _ = app.emit("helper", json!({"from": "index", "event": event})); } +/// The exits that mean "the helper died on the file it was reading": the fatal signals on Unix (SIGILL, +/// SIGTRAP — Swift's allocation trap, the "exited (5)" of #182 — SIGABRT, SIGFPE, SIGKILL, SIGBUS, SIGSEGV), +/// where a child killed by a signal has no exit code at all and the signal carries the class, and the +/// Windows SEH crash codes (breakpoint, access violation, stack overflow, fast fail, illegal instruction, +/// private instruction, divide-by-zero, in-cell divide). `status` alone is inert on Linux (it is None there, +/// so `as_i64().unwrap_or(0)` answers 0): the pre-push review caught that, and the Mac app's +/// `terminationStatus` has always carried the signal. The `CrashNotes` net on Windows covers only the SEH +/// classes the C runtime translates there, so a note may be absent — the fallback then marks nothing, and +/// the restart cap bounds the loop (disclosed Windows limit). +fn died_on_file(status: i64, signal: Option) -> bool { + matches!(signal, Some(4 | 5 | 6 | 8 | 9 | 10 | 11)) + || matches!(status, 4 | 5 | 6 | 8 | 9 | 10 | 11 + | 0x80000004 | 0xC0000005 | 0xC00000FD | 0xC0000409 | 0xC000001D | 0xC0000094 | 0xC0000096) +} + /// The index helper ended by itself (helper.rs's "ended"). A sync asked of it as it was leaving (it exits a minute after /// it's done) still has to happen; its watchdog's exit (75: a file whose reading ran away, which the next helper skips) /// starts another, though not over and over if something else is wrong (as the Mac app's `Engine.helperExited`). fn index_ended(app: &AppHandle, event: &Value) { const STOPPED_ON_FILE: i64 = 75; let shell = app.state::>().inner().clone(); - let again = if event["status"].as_i64() == Some(STOPPED_ON_FILE) { + let status = event["status"].as_i64().unwrap_or(0); + let signal = event["signal"].as_i64(); + let died_on_file = died_on_file(status, signal); + let again = if status == STOPPED_ON_FILE || died_on_file { + if died_on_file { + eprintln!("digup-shell: the helper died (exit {status}, signal {signal:?}) on a file: it will be \ + marked, and indexing goes on"); + *shell.crash_file.lock().unwrap() = shell.last_file.lock().unwrap().clone(); + } let mut stops = shell.watchdog_stops.lock().unwrap(); stops.retain(|stop| stop.elapsed() < Duration::from_secs(600)); stops.push(Instant::now()); + // 100 files moved since the last stop means the stops are getting past bad files, not looping + // (#182: the 4-stops cap froze a 55,000-file index after four oversized RAWs). + let pending = shell.status.lock().unwrap().pending as i64; + let before = shell.pending_at_stop.swap(pending, Ordering::SeqCst); + if before - pending >= 100 { + stops.clear(); + stops.push(Instant::now()); + } if stops.len() > 3 { eprintln!("digup-shell: indexing stopped on files 4 times in 10 min"); } stops.len() <= 3 } else { - event["owed"] == true && event["status"].as_i64() == Some(0) + event["owed"] == true && status == 0 }; if again { run_index(app); @@ -2025,3 +2084,33 @@ mod tests { assert_eq!(fixed, "[Desktop Entry]\nType=Application\nExec=\"/home/me/.local/opt/digup/AppRun\" --at-login\nName=DigUp\n"); } } + +#[cfg(test)] +mod crash_class_tests { + use super::died_on_file; + + /// Issue #182's crash classification, in both platforms' shapes. On Unix a child killed by a signal has + /// no exit code at all (`ExitStatus::code()` is None there, so `status` reaches us as 0) and the signal + /// carries the class; the Mac app's `terminationStatus` spells the same crash as the status itself. + /// Clean exits (0), the watchdog's own stop (75), and a plain nonzero exit are not crash classes. + #[test] + fn fatal_signals_carry_the_crash_class() { + for signal in [4, 5, 6, 8, 9, 10, 11] { + assert!(died_on_file(0, Some(signal)), "signal {signal}"); + assert!(died_on_file(signal, None), "macOS spells it as the status"); + } + assert!(!died_on_file(0, Some(13))); // SIGPIPE: not a crash on a file + assert!(!died_on_file(0, None)); // an exit, not a killing + assert!(!died_on_file(75, None)); // the watchdog's own stop (EX_TEMPFAIL) + assert!(!died_on_file(1, None)); // an ordinary failure exit + } + + /// Windows has no signals: the crash class arrives as the SEH exit code (a Swift trap, a CRT fast-fail). + #[test] + fn seh_codes_carry_the_crash_class() { + for code in [0x80000004i64, 0xC0000005, 0xC00000FD, 0xC0000409, 0xC000001D, 0xC0000094, 0xC0000096] { + assert!(died_on_file(code, None), "code {code:#x}"); + } + assert!(!died_on_file(0xC0000022, None)); // a share-stop code: an exit, not a crash + } +}