From 1ba9b59e8a7a82944d368d1f76f1d7d0d2ac72f1 Mon Sep 17 00:00:00 2001 From: OmikronApex Date: Tue, 14 Jul 2026 12:16:22 +0200 Subject: [PATCH] fix(audio): validate LoadPreset arg before arming the rebuild barrier MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PR #107 round-2 review: beginChainRebuild() ran before info[0].As(), so a non-string argument threw between begin and the worker taking ownership — leaking the barrier and blocking editor opens permanently. Validate + read the argument first; the barrier is now armed only on a path where every exit releases it. Co-Authored-By: Claude Fable 5 --- src/audio/addon/ChainOps.cpp | 21 ++++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-) diff --git a/src/audio/addon/ChainOps.cpp b/src/audio/addon/ChainOps.cpp index 36e8f1b..98ae709 100644 --- a/src/audio/addon/ChainOps.cpp +++ b/src/audio/addon/ChainOps.cpp @@ -886,12 +886,28 @@ Napi::Value LoadPreset(const Napi::CallbackInfo& info) return deferred.Promise(); } + // Validate + read the argument BEFORE arming the barrier: with a + // non-string arg, As() throws (JS TypeError / C++ + // exception), and anything thrown between begin and the worker taking + // ownership would leak the barrier and block editor opens forever. + if (!info[0].IsString()) + { + auto obj = Napi::Object::New(env); + obj.Set("success", false); + obj.Set("error", "preset must be a JSON string"); + deferred.Resolve(obj); + return deferred.Promise(); + } + auto json = info[0].As().Utf8Value(); + // Arm the rebuild barrier BEFORE editor teardown: between closeAll…() // returning and the queued worker acquiring chainMutationMutex, nothing // else stops OpenPluginEditor from opening a fresh editor whose processor // the worker is about to free (#56). The barrier gates editor opens for // the whole teardown+rebuild window; the worker releases it on every - // Execute() exit path. + // Execute() exit path. Nothing between here and Queue() can throw: the + // teardown-failure path below releases explicitly, and the worker's + // BarrierRelease guard covers every Execute() exit. slopsmith::addon::beginChainRebuild(); // Tear down any open in-process editor windows NOW, on the N-API/main @@ -915,8 +931,7 @@ Napi::Value LoadPreset(const Napi::CallbackInfo& info) return deferred.Promise(); } - auto json = info[0].As().Utf8Value(); - auto worker = new LoadPresetWorker(env, deferred, json); + auto worker = new LoadPresetWorker(env, deferred, std::move(json)); worker->Queue(); return deferred.Promise(); }