From 57ce1fe08dd46cf4c9e708d988896a5b42d26c74 Mon Sep 17 00:00:00 2001 From: dsward2 Date: Tue, 22 Sep 2026 22:07:45 -0500 Subject: [PATCH] Fix SIGABRT on shutdown: serialize encoder encode/flush/stop LiveAudioServer.stop() (a Swift task) stopped the MP3 and AAC encoders while the stdin-pcm-reader thread was still encoding, sending its EOF flush, and then calling stop() again in its own epilogue. Nothing serialized these, so AudioConverterReset/Dispose and lame_encode_flush/ lame_close ran concurrently on the same handle: "pointer being freed was not allocated" in aacClose, or a LAME assert in format_bitstream. - AACEncoder / MP3Encoder: an NSLock serializes encode, flush and stop; stop() is idempotent and later encode() calls are no-ops. - AACEncoder.stop() no longer resets before disposing (reset only discards buffered input and emits nothing). - MP3Encoder flushes at most once (EOF flush and stop no longer both run). - CLI: graceful shutdown is one-shot, so the --exit-with-parent watchdog (fires every 0.5 s) can't start a second shutdown that exit(0)s while the first is still tearing down. Co-Authored-By: Claude Opus 5.5 --- .../LiveAudioServer/LiveAudioServerApp.swift | 11 ++++++++++ Sources/LiveAudioServerCore/AACEncoder.swift | 19 +++++++++++++++++- Sources/LiveAudioServerCore/MP3Encoder.swift | 20 +++++++++++++++++-- 3 files changed, 47 insertions(+), 3 deletions(-) diff --git a/Sources/LiveAudioServer/LiveAudioServerApp.swift b/Sources/LiveAudioServer/LiveAudioServerApp.swift index 44a555a..9367d0d 100644 --- a/Sources/LiveAudioServer/LiveAudioServerApp.swift +++ b/Sources/LiveAudioServer/LiveAudioServerApp.swift @@ -203,7 +203,18 @@ struct LiveAudioServerApp { // Graceful shutdown wired to SIGINT/SIGTERM. Ignoring the kernel // default first means the DispatchSourceSignal sees the signal // instead of the process being torn down. + // One-shot: the parent watchdog keeps firing every 0.5 s after the + // parent dies, and a second SIGTERM can land mid-teardown. A repeat + // call would see stop() return early and exit(0) while the first + // shutdown is still disposing encoders. + let shutdownLock = NSLock() + var shutdownStarted = false let runGracefulShutdown: (String) -> Void = { reason in + shutdownLock.lock() + let alreadyStarted = shutdownStarted + shutdownStarted = true + shutdownLock.unlock() + if alreadyStarted { return } log("\n\(reason) received — graceful shutdown") Task { await server.stop() diff --git a/Sources/LiveAudioServerCore/AACEncoder.swift b/Sources/LiveAudioServerCore/AACEncoder.swift index ca0adf5..7592837 100644 --- a/Sources/LiveAudioServerCore/AACEncoder.swift +++ b/Sources/LiveAudioServerCore/AACEncoder.swift @@ -71,6 +71,14 @@ final class AACEncoder { private var outputBuf: [UInt8] + /// Serializes encode / flush / stop. The PCM reader thread encodes (and + /// flushes on EOF) while `LiveAudioServer.stop()` tears down from a Swift + /// task; without this, AudioConverterReset/Dispose ran concurrently on the + /// same converter and crashed in aacClose ("pointer being freed was not + /// allocated"). + private let lock = NSLock() + private var isStopped = false + init(config: AudioEncoderConfig, onEncoded: @escaping (Data) -> Void) { self.config = config self.onEncoded = onEncoded @@ -112,8 +120,13 @@ final class AACEncoder { encoderLog("AAC encoder ready: \(config.aacBitrate/1000)kbps, \(config.channels)ch, \(config.sampleRate)Hz", config: config) } + /// Idempotent. Disposes the converter exactly once; later `encode` calls + /// are no-ops. No reset first: AudioConverterReset only discards buffered + /// input (it emits nothing), and Dispose frees the codec anyway. func stop() { - flush() + lock.lock(); defer { lock.unlock() } + guard !isStopped else { return } + isStopped = true if let conv = converter { AudioConverterDispose(conv) converter = nil @@ -123,6 +136,8 @@ final class AACEncoder { // MARK: - Encoding func encode(samples: UnsafeBufferPointer) { + lock.lock(); defer { lock.unlock() } + guard !isStopped else { return } if samples.count == 0 { flush() return @@ -133,6 +148,7 @@ final class AACEncoder { // MARK: - Internal + /// Caller must hold `lock`. private func drainQueue() { guard let converter = converter else { return } let samplesNeeded = framesPerPacket * config.channels @@ -200,6 +216,7 @@ final class AACEncoder { } } + /// Caller must hold `lock`. private func flush() { if let conv = converter { AudioConverterReset(conv) diff --git a/Sources/LiveAudioServerCore/MP3Encoder.swift b/Sources/LiveAudioServerCore/MP3Encoder.swift index a475706..e996583 100644 --- a/Sources/LiveAudioServerCore/MP3Encoder.swift +++ b/Sources/LiveAudioServerCore/MP3Encoder.swift @@ -9,6 +9,14 @@ final class MP3Encoder { private var lame: lame_t? private var mp3Buf: [UInt8] + /// Serializes encode / flush / stop. The PCM reader thread encodes (and + /// flushes on EOF) while `LiveAudioServer.stop()` tears down from a Swift + /// task; without this, lame_encode_flush / lame_close ran concurrently on + /// the same handle and aborted inside LAME. + private let lock = NSLock() + private var isStopped = false + private var isFlushed = false + // LAME recommends output buffer = 1.25 * samples + 7200 private var mp3BufSize: Int { Int(Double(config.chunkFrames) * 1.25) + 7200 } @@ -38,7 +46,12 @@ final class MP3Encoder { encoderLog("MP3 encoder ready: \(config.mp3Bitrate)kbps, \(config.channels)ch, \(config.sampleRate)Hz", config: config) } + /// Idempotent. Flushes (if the EOF path hasn't already) and closes LAME + /// exactly once; later `encode` calls are no-ops. func stop() { + lock.lock(); defer { lock.unlock() } + guard !isStopped else { return } + isStopped = true flush() if lame != nil { lame_close(lame) @@ -50,7 +63,8 @@ final class MP3Encoder { /// Called with each PCM chunk. count==0 signals EOF/flush. func encode(samples: UnsafeBufferPointer) { - guard let lame = lame else { return } + lock.lock(); defer { lock.unlock() } + guard !isStopped, let lame = lame else { return } if samples.count == 0 { flush() @@ -105,8 +119,10 @@ final class MP3Encoder { // MARK: - Private + /// Caller must hold `lock`. Runs at most once per encoder. private func flush() { - guard let lame = lame else { return } + guard !isFlushed, let lame = lame else { return } + isFlushed = true var flushBuf = [UInt8](repeating: 0, count: 7200) let n = flushBuf.withUnsafeMutableBytes { ptr in lame_encode_flush(lame,