From 0ace3e4111fd8e5538a0f35a41bc80ce01ddf819 Mon Sep 17 00:00:00 2001 From: NimBold Date: Wed, 10 Jun 2026 23:43:10 +0330 Subject: [PATCH] fix: resolve additional bugs identified in code review - core: remove DispatchGroup from MediaExtractionEngine to fix hanging on yt-dlp launch failure - core: clear readabilityHandlers in Aria2DownloadEngine if process fails to launch - core: fix TOCTOU type warning for findFreePort - build: fix inverted logic in Mach-O validation script - ci: fix hardcoded macOS SDK version check in release workflow - ci: prevent workflow_dispatch from failing on verify-tag --- .github/workflows/release.yml | 14 +++++++++----- Scripts/create_app_bundle.sh | 2 +- Sources/Firelink/Aria2DownloadEngine.swift | 8 +++++--- Sources/Firelink/MediaExtractionEngine.swift | 20 ++++++-------------- 4 files changed, 21 insertions(+), 23 deletions(-) diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index c324dc7..f94bc2c 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -17,7 +17,7 @@ permissions: jobs: macos-arm64-dmg: name: Build macOS ARM64 DMG - runs-on: macos-26 + runs-on: macos-14 steps: - name: Check out repository @@ -49,14 +49,14 @@ jobs: xcode-select -p xcrun --sdk macosx --show-sdk-version - - name: Verify macOS 26 SDK + - name: Verify macOS 14+ SDK shell: bash run: | SDK_VERSION="$(xcrun --sdk macosx --show-sdk-version)" SDK_MAJOR="${SDK_VERSION%%.*}" - if [[ "$SDK_MAJOR" != "26" ]]; then - echo "Expected macOS 26 SDK, got macOS $SDK_VERSION" >&2 + if [[ "$SDK_MAJOR" -lt 14 ]]; then + echo "Expected at least macOS 14 SDK, got macOS $SDK_VERSION" >&2 exit 1 fi @@ -123,5 +123,9 @@ jobs: gh release upload "$TAG_NAME" dist/*.dmg --clobber gh release edit "$TAG_NAME" --notes-file release_notes.md else - gh release create "$TAG_NAME" dist/*.dmg --title "$TAG_NAME" --notes-file release_notes.md --verify-tag + if [[ "${{ github.event_name }}" == "workflow_dispatch" ]]; then + gh release create "$TAG_NAME" dist/*.dmg --title "$TAG_NAME" --notes-file release_notes.md + else + gh release create "$TAG_NAME" dist/*.dmg --title "$TAG_NAME" --notes-file release_notes.md --verify-tag + fi fi diff --git a/Scripts/create_app_bundle.sh b/Scripts/create_app_bundle.sh index bef3b70..2d0bd48 100755 --- a/Scripts/create_app_bundle.sh +++ b/Scripts/create_app_bundle.sh @@ -20,7 +20,7 @@ cd "$ROOT_DIR" is_valid_mach_o() { local path="$1" if ! file "$path" | grep -q 'Mach-O'; then - return 0 + return 1 fi lipo -archs "$path" >/dev/null 2>&1 diff --git a/Sources/Firelink/Aria2DownloadEngine.swift b/Sources/Firelink/Aria2DownloadEngine.swift index 78fead7..9ed1766 100644 --- a/Sources/Firelink/Aria2DownloadEngine.swift +++ b/Sources/Firelink/Aria2DownloadEngine.swift @@ -10,7 +10,7 @@ final class Aria2DownloadEngine: Sendable { let cancel: @Sendable () -> Void } - static func findFreePort() -> Int { + static func findFreePort() -> UInt16 { var port: UInt16 = 6800 let parameters = NWParameters.tcp for p in 6800...6900 { @@ -21,7 +21,7 @@ final class Aria2DownloadEngine: Sendable { break } } - return Int(port) + return port } enum EngineError: LocalizedError { @@ -120,7 +120,7 @@ final class Aria2DownloadEngine: Sendable { var lastError: Error? for _ in 1...5 { - let rpcPort = Self.findFreePort() + let rpcPort = Int(Self.findFreePort()) let rpcSecret = UUID().uuidString let tempDir = URL(fileURLWithPath: NSTemporaryDirectory()).appendingPathComponent("firelink-aria2-\(UUID().uuidString)") @@ -221,6 +221,8 @@ final class Aria2DownloadEngine: Sendable { } if didThrow { + outputPipe.fileHandleForReading.readabilityHandler = nil + errorPipe.fileHandleForReading.readabilityHandler = nil try? FileManager.default.removeItem(at: tempDir) continue } diff --git a/Sources/Firelink/MediaExtractionEngine.swift b/Sources/Firelink/MediaExtractionEngine.swift index e42720e..799e4ae 100644 --- a/Sources/Firelink/MediaExtractionEngine.swift +++ b/Sources/Firelink/MediaExtractionEngine.swift @@ -416,16 +416,10 @@ private final class YTDLPMetadataProcess: @unchecked Sendable { process.standardError = errorPipe process.standardInput = nil - let group = DispatchGroup() - group.enter() // output - group.enter() // error - group.enter() // process - outputPipe.fileHandleForReading.readabilityHandler = { handle in let data = handle.availableData if data.isEmpty { handle.readabilityHandler = nil - group.leave() } else { outputBuffer.append(data) } @@ -435,7 +429,6 @@ private final class YTDLPMetadataProcess: @unchecked Sendable { let data = handle.availableData if data.isEmpty { handle.readabilityHandler = nil - group.leave() } else { errorBuffer.append(data) } @@ -445,12 +438,11 @@ private final class YTDLPMetadataProcess: @unchecked Sendable { self.process = process } - process.terminationHandler = { _ in - group.leave() - } - - group.notify(queue: .global()) { - if process.terminationStatus == 0 { + process.terminationHandler = { finishedProcess in + outputPipe.fileHandleForReading.readabilityHandler = nil + errorPipe.fileHandleForReading.readabilityHandler = nil + + if finishedProcess.terminationStatus == 0 { continuation.resume(returning: outputBuffer.data) } else { let stderr = String(data: errorBuffer.data, encoding: .utf8)? @@ -463,7 +455,7 @@ private final class YTDLPMetadataProcess: @unchecked Sendable { .joined(separator: "\n") continuation.resume( throwing: MediaExtractionEngine.ExtractionError.processFailed( - message.isEmpty ? "Exit code \(process.terminationStatus)" : message + message.isEmpty ? "Exit code \(finishedProcess.terminationStatus)" : message ) ) }