Skip to content

3.13.2 — Bug-audit hardening (concurrency, memory safety) and O(log n) segment lookup

Choose a tag to compare

@superuser404notfound superuser404notfound released this 23 Jun 10:43

Fixed

AVIOReader close-flag data race on persistent-connection teardown

The persistent-connection AVIOReader kept its isClosed and isFullyClosed state in bare Bools. markClosed() / fullyClose() wrote them on the teardown thread (Demuxer.close via SoftwarePlaybackHost.stop) while the URLSession delegate threads read them in appendPersistentData(), a ThreadSanitizer-confirmed cross-thread race on torn access. Both flags are now backed by a leaf OSAllocatedUnfairLock so every get/set is synchronized; the lock is only ever held for the access itself, never across winCond/streamLock, so it cannot invert. Found off-device via TSan while verifying the audit fixes and confirmed pre-existing (it reproduces identically on main with none of the other audit fixes present), so not a regression; the fix also clears the SIGABRT TSan raised on DVR teardown.

AudioBridge mutators raced on a slow-source producer restart

On a producer restart whose old pump did not exit within the 5s budget (slow or remote source), the abandoned pump could still be inside feed() while the restart thread called startSegment() on the otherwise lock-free AudioBridge. Concurrent libswresample / libavcodec calls on the shared resampler, encoder, and FIFO are a data race (corrupt audio or a libav crash). The public mutators (feed/flush/startSegment/noteTimelineJump/close) now serialize through an internal NSLock, mirroring AudioDecoder.stateLock. The lock is uncontended in the normal single-pump case and is only ever held during CPU-bound encode work, never across the demuxer's blocking read, so it cannot deadlock the restart's waitForFinish. Each mutator already guards or tolerates freed contexts, so a feed() that loses the race to a concurrent close() returns empty instead of touching freed memory; the lock-free diagnostic reads (fifoSampleCount/liveBytes) are foreclosed by the engine lifecycle.

DVR rewind could seed the feeder mid-GOP

seekLiveDVR seeded the feeder cursor at ring.seqBounds.first when no keyframe existed at or before the target. seqBounds.first is firstSeq, the oldest retained entry, which before the ring's first eviction is not guaranteed to be a keyframe (a mid-GOP live join), so the feeder could start at an inter-frame and emit decode garbage until the next keyframe. A new PacketRingBuffer.firstKeyframeSeq() returns the earliest retained keyframe, and the reseed now falls to it before seqBounds.first.

Audio decoder dropped the final tail at EOF

AudioDecoder coalesces PCM and only emits once at least 1024 samples accumulate, but it had no drain path, and AudioPlaybackHost called flush() (avcodec_flush_buffers + reset) at demuxer EOF, discarding both the decoder-delay frames and the sub-threshold residual. The final ~21ms or more of every audio-only FFmpeg-path title was dropped. A new AudioDecoder.drain() (NULL packet, receive remaining frames, force-emit the residual, mirroring AudioBridge.flush()) now runs at EOF; its output is enqueued and the playthrough high-water mark extended before flush().

Native subtitle cue stores raced the pump thread

nativeSubtitleCueStoresForSession and the parallel languages array were plain vars reassigned by attachNativeSubtitleStores on the host thread while the pump thread iterated them in handleVideoShiftKnown and makeProducer read them; iterating the live array during a CoW reassignment is a data race. The arrays are now written under restartLock in the attach path and snapshotted under restartLock before iteration in handleVideoShiftKnown, mirroring the file's existing subsystem-ref snapshot discipline. Gated on opt-in prepareNativeSubtitles plus a text track.

SMB cancel() waited out the libsmb2 timeout instead of unblocking the read

SMBIOReader.read() blocks the demux thread on a semaphore while AMSMB2 runs the libsmb2 read, which is not cancellation-aware (only an internal ~60s timeout resolves the continuation). cancel() called only task.cancel(), which cannot interrupt the running read, so teardown blocked up to 60s. The per-read semaphore is now published alongside the task so cancel() signals it directly; read() then aborts with -1 without touching the outcome (the background Task may still be writing it, and the semaphore's happens-before edge only covers the Task's own signal), and the libsmb2 op drains in the background. This is the engine-side completion of the ConcatIOReader.cancel() forwarding below.

Superseded live seek could resurrect a torn-down session

The isLive branch of seek(to:) wrote clock.currentTime/sourceTime and state = .playing after an async host seek while guarding only on seekGeneration. A concurrent stop()/load()/zap during the await bumps loadGeneration (in stopInternal) but leaves seekGeneration untouched, so the guard passed and a superseded live seek wrote playback state onto a torn-down or successor session. loadGeneration is now captured before the isLive branch and checked at both live finalize points (SW and native), matching the guard the VOD finalize already used.

Partial encoded packets leaked when feed()'s FIFO drain threw

drainFIFOIntoEncoder appends encoded AVPackets (trackedPacketAlloc) to its results across loop iterations and can throw sendFrameFailed after some are already appended. feed() propagated that throw with try and never returned results, so every packet already appended leaked (PacketBalanceTracker.alive climbs) since the caller logs and continues holding no reference. The drain is now wrapped so the partial results are freed before rethrowing. flush() intentionally keeps its partial results via try?, so the cleanup lives in feed(), not the shared helper.

FrameExtractor lost last-GOP snapshots at EOF

decodeFrame returned nil the moment the demuxer hit EOF, without ever sending the decoder a NULL flush packet. The context is frame-threaded (FF_THREAD_FRAME), so several frames stay buffered and only emit after a flush, and a snapshot or thumbnail targeting the final frames of a stream came back nil (blank scrub preview). On EOF the decoder is now flushed with a NULL packet and drained until EOF, returning the first frame at or after the target; drain mode terminates on EAGAIN / error / EOF to avoid re-flush loops.

Untrusted UDF extent allocation capped in DiscReader.readAll

readAll summed .mpls allocation-extent lengths (untrusted on-disc bytes, up to ~1 GB each) and allocated that many bytes with no upper bound, and passed the running total into Int32 read sizes, which could trap above Int32.max. A crafted Blu-ray ISO could drive an arbitrary allocation (jetsam / DoS). The total is now capped at 8 MB before allocating (matching UDFReader.readDirectory's existing guard, and far above any real KB-scale playlist) and the per-read size is clamped.

ConcatIOReader.cancel() did not reach the base reader

ConcatIOReader implemented read/seek/close/makeIndependentReader but not cancel(), so it inherited the protocol's no-op default. The engine's teardown path (CustomIOReaderBridge.markClosed) calls cancel() on the ConcatIOReader, not the inner base, so a read parked inside base.read() on a network-backed disc source (an SMB-served ISO, for example) was never unblocked and teardown hung. cancel() now forwards to the base, mirroring the existing makeIndependentReader.

Double-free of AVFormatContext on sidecar HTTP open failure

In decodeFileSync's HTTP branch, formatContext was assigned the freshly allocated context before avformat_open_input ran. On open failure FFmpeg frees that context and NULLs the pointer it was given, but formatContext still held the now-freed pointer, so the defer's avformat_close_input double-freed it. formatContext is now assigned only after the open succeeds, mirroring the local-file branch and Demuxer.swift. Reachable via any HTTP/HTTPS sidecar subtitle whose AVIO opens but whose container open fails (empty or malformed body, partial 2xx).

Performance

O(log n) segment index lookup on the pump path

segmentIndex(forSourcePts:) ran a linear "first i where absolute < boundaries[i+1]" scan on every video and audio packet; the walk length grew with elapsed playback (roughly one compare per segment, hundreds deep late in a VOD title), the only genuinely super-linear per-packet operation on the pump path. The index math is extracted into a pure static segmentOffset(forAbsolutePts:boundaries:) using an upper-bound binary search, exactly equivalent to the old scan and clamped identically to [0, count-2]. Equivalence is proven by SegmentOffsetTests, which cross-checks the binary search against a reference linear scan over a deterministic sweep (edges, exact boundaries, adjacent boundaries, out-of-range, long VOD-like layouts).

Internal

  • Cleared 27 strict-concurrency and deprecation warnings across two passes (17 in the first, the final 10 by async-refactoring NativeAVPlayerHost's diagnostic dumps onto @MainActor Tasks dispatched off the KVO / observer callbacks), with no behavior change.
  • Removed dead code, write-only / unread state, and redundant indirection across audio, video, demuxer, network, disc, renderer, native, and subtitles (unread isHDR and currentlyHDR flags, liveExhausted, seg0FetchTime and a provider pass-through, an unused audio enum case and pass-through wrapper, inlined single-call rect-text passes, a stale stalled flag in HLSLiveRepro, collapsed redundant togglePlayPause host branches, shared FrameExtractor stored-property init via a private designated init).
  • Condensed and pruned redundant code comments across the codebase (aetherctl, core types, Video muxer / cache, Disc, Demuxer, Native, Display, Network, Renderer, View, SMB).
  • Narrowed DiscFile / DiscError from public to internal (only used by @testable tests, not part of any public surface).
  • Expanded test coverage: FragmentSplitter box-split, SegmentCache window / prune index math, and the segmentOffset binary-search equivalence sweep.
  • Documented the dual-subtitle API and the dualsubs CLI; corrected subcommand counts and an AV1 claim in the docs.
  • Added KSPlayer to the README "how it compares" table.

Full diff: 3.13.1...3.13.2