From 05e4244cc8877c56780fb9a6be6703cc152b6a30 Mon Sep 17 00:00:00 2001 From: locphamtran Date: Thu, 13 Aug 2026 15:32:35 +0700 Subject: [PATCH] fix(bridge): one editor at a time + guard reload race - Close other channels' editor windows before attaching a new one (Option B) - needsReload()/setReloading() guard: skip processAudioBlock during terminate+reload teardown to avoid UAF/hang when reopening GUI - Wait for editor registry cleanup (5s timeout) before creating window --- native_bridge/include/INativeInstrument.h | 7 + .../include/NativeInstrumentEngine.h | 5 + native_bridge/include/Vst3Instrument.h | 3 + native_bridge/src/NativeInstrumentEngine.cpp | 6 + native_bridge/src/Vst3Instrument.cpp | 8 +- native_bridge/src/gui_probe.cpp | 269 +++++++++++++++++- native_bridge/src/main.cpp | 55 +++- 7 files changed, 349 insertions(+), 4 deletions(-) diff --git a/native_bridge/include/INativeInstrument.h b/native_bridge/include/INativeInstrument.h index 7859622..6f3055f 100644 --- a/native_bridge/include/INativeInstrument.h +++ b/native_bridge/include/INativeInstrument.h @@ -34,6 +34,13 @@ public: // Open/close Native GUI window virtual bool openGUI(void* parentWindowHandle) = 0; virtual void closeGUI() = 0; + // VST3 reopen-after-close: the bridge checks before openGUI whether the + // instance must be rebuilt; while it is, the engine sets reloading=true + // and the real-time loop skips processAudioBlock (teardown must not race + // the audio thread — a reload terminates the VST while renderAll may be + // inside processAudioBlock on the same instance). + virtual bool needsReload() const { return false; } + virtual void setReloading(bool /*on*/) {} // Real-time Audio PCM Float32 rendering loop virtual void processAudioBlock(float* outputL, float* outputR, uint32_t numSamples) = 0; diff --git a/native_bridge/include/NativeInstrumentEngine.h b/native_bridge/include/NativeInstrumentEngine.h index b40c3ec..00172f5 100644 --- a/native_bridge/include/NativeInstrumentEngine.h +++ b/native_bridge/include/NativeInstrumentEngine.h @@ -72,6 +72,11 @@ public: INativeInstrument* get(uint32_t channel); + // Mark a channel as rebuilding its VST instance (GUI reopen): the + // real-time loop then skips processAudioBlock for that channel so VST + // teardown never races the audio thread. + void setReloading(uint32_t channel, bool on); + // Flush every sounding note on every assigned channel. void allNotesOff(); diff --git a/native_bridge/include/Vst3Instrument.h b/native_bridge/include/Vst3Instrument.h index 40235b5..1d2f128 100644 --- a/native_bridge/include/Vst3Instrument.h +++ b/native_bridge/include/Vst3Instrument.h @@ -26,6 +26,8 @@ public: void pitchBend(uint32_t channel, uint32_t bend14) override; bool openGUI(void* parentWindowHandle) override; void closeGUI() override; + bool needsReload() const override { return hasAttachedOnce_ && !guiAttached_; } + void setReloading(bool on) override { reloading_ = on; } void processAudioBlock(float* outputL, float* outputR, uint32_t numSamples) override; private: @@ -37,6 +39,7 @@ private: bool loaded_; bool guiAttached_; bool hasAttachedOnce_; + bool reloading_; // guarded by InstrumentEngineManager::mu_ (set via setReloading) bool reload(); }; diff --git a/native_bridge/src/NativeInstrumentEngine.cpp b/native_bridge/src/NativeInstrumentEngine.cpp index 3d290fb..78c5b04 100644 --- a/native_bridge/src/NativeInstrumentEngine.cpp +++ b/native_bridge/src/NativeInstrumentEngine.cpp @@ -195,6 +195,12 @@ INativeInstrument* InstrumentEngineManager::get(uint32_t channel) { return it == channels_.end() ? nullptr : it->second.get(); } +void InstrumentEngineManager::setReloading(uint32_t channel, bool on) { + std::lock_guard lock(mu_); + auto it = channels_.find(channel); + if (it != channels_.end()) it->second->setReloading(on); +} + void InstrumentEngineManager::allNotesOff() { std::lock_guard lock(mu_); for (auto& [ch, inst] : channels_) { diff --git a/native_bridge/src/Vst3Instrument.cpp b/native_bridge/src/Vst3Instrument.cpp index d8bb647..487c116 100644 --- a/native_bridge/src/Vst3Instrument.cpp +++ b/native_bridge/src/Vst3Instrument.cpp @@ -170,7 +170,8 @@ Vst3Instrument::Vst3Instrument() maxBlockSize_(256), loaded_(false), guiAttached_(false), - hasAttachedOnce_(false) {} + hasAttachedOnce_(false), + reloading_(false) {} Vst3Instrument::~Vst3Instrument() { #ifdef HAVE_VST3SDK @@ -598,6 +599,11 @@ void Vst3Instrument::processAudioBlock(float* outputL, float* outputR, uint32_t #ifndef HAVE_VST3SDK return; #else + // GUI reopen rebuilds the instance on the worker thread; while the + // teardown runs, the audio loop must not touch state_ (flag toggled under + // InstrumentEngineManager::mu_ by setReloading — renderAll holds the same + // mutex during processAudioBlock, so the check is race-free). + if (reloading_) return; auto* s = static_cast(state_); if (!s || !s->component || numSamples == 0) return; FUnknownPtr processor(s->component); diff --git a/native_bridge/src/gui_probe.cpp b/native_bridge/src/gui_probe.cpp index 2b7ace9..bac5ba2 100644 --- a/native_bridge/src/gui_probe.cpp +++ b/native_bridge/src/gui_probe.cpp @@ -9,8 +9,12 @@ #endif #include #include +#include #include #include +#include +#include +#include #include #include @@ -29,10 +33,54 @@ static void pump_for(int secs) { static void sleep_for(int secs) { Sleep((DWORD)(secs * 1000)); } +// Per-channel persistent worker replicating the bridge (round-3 probe). +class ProbeWorker { +public: + void start() { + th = std::thread([this] { +#ifdef _WIN32 + OleInitialize(nullptr); +#endif + std::unique_lock lk(mu); + for (;;) { + if (stop && jobs.empty()) break; + if (!jobs.empty()) { + auto j = std::move(jobs.front()); + jobs.pop_front(); + lk.unlock(); + j(); + lk.lock(); + continue; + } + cv.wait_for(lk, std::chrono::milliseconds(5)); + lk.unlock(); + MSG msg; + while (PeekMessageW(&msg, nullptr, 0, 0, PM_REMOVE)) { + TranslateMessage(&msg); + DispatchMessageW(&msg); + } + lk.lock(); + } +#ifdef _WIN32 + OleUninitialize(); +#endif + }); + } + void post(std::function j) { + { std::lock_guard lk(mu); jobs.push_back(std::move(j)); } + cv.notify_all(); + } + std::thread th; + std::mutex mu; + std::condition_variable cv; + std::deque> jobs; + bool stop = false; +}; + int main(int argc, char* argv[]) { if (argc < 2) { printf("usage: gui_probe [variant] [secs] [hwnd]\n" - "variants: main_own | worker_own_nopump | worker_own_pump | worker_foreign_nopump | worker_foreign_pump | bridge_like | same_thread\n"); + "variants: main_own | worker_own_nopump | worker_own_pump | worker_foreign_nopump | worker_foreign_pump | bridge_like | same_thread | two_instances | two_instances_close | close_reopen | bridge_two | shared_worker\n"); return 2; } std::string path = argv[1]; @@ -68,7 +116,11 @@ int main(int argc, char* argv[]) { printf("loading %s\n", path.c_str()); fflush(stdout); Vst3Instrument inst; - bool isDeferred = (variant == "bridge_like" || variant == "same_thread"); + Vst3Instrument inst2; + bool isDeferred = (variant == "bridge_like" || variant == "same_thread" || + variant == "two_instances" || variant == "two_instances_close" || + variant == "close_reopen" || variant == "bridge_two" || + variant == "shared_worker"); bool ok = true; if (!isDeferred) { ok = inst.loadPlugin(path, 44100.0); @@ -131,6 +183,219 @@ int main(int argc, char* argv[]) { }); pump_for(secs); if (!done.load()) { printf("RESULT: TIMEOUT (same_thread)\n"); return 3; } + } else if (variant == "shared_worker") { + // Proposed round-3 fix: ALL VST3 instances live in ONE apartment — + // one persistent worker thread hosts load + openGUI for every channel. + ProbeWorker w; + w.start(); + std::atomic done{false}; + int aRes = 0, bRes = 0; + w.post([&]() { + bool r = inst.loadPlugin(path, 44100.0); + printf("[shared_worker] load inst1=%d\n", r ? 1 : 0); + fflush(stdout); + if (r) { + HWND h = CreateWindowEx(0, "GuiProbeClass", "ProbeA", WS_OVERLAPPEDWINDOW, + 0, 0, 800, 600, nullptr, nullptr, GetModuleHandle(nullptr), nullptr); + r = inst.openGUI(h); + printf("[shared_worker] attach inst1=%d hwnd=%p\n", r ? 1 : 0, (void*)h); + fflush(stdout); + aRes = r ? 1 : 0; + } + r = inst2.loadPlugin(path, 44100.0); + printf("[shared_worker] load inst2=%d\n", r ? 1 : 0); + fflush(stdout); + if (r) { + HWND h = CreateWindowEx(0, "GuiProbeClass", "ProbeB", WS_OVERLAPPEDWINDOW, + 0, 0, 800, 600, nullptr, nullptr, GetModuleHandle(nullptr), nullptr); + r = inst2.openGUI(h); + printf("[shared_worker] attach inst2=%d hwnd=%p\n", r ? 1 : 0, (void*)h); + fflush(stdout); + bRes = r ? 1 : 0; + } + done = true; + }); + auto t0 = std::chrono::steady_clock::now(); + while (!done.load() && std::chrono::duration_cast( + std::chrono::steady_clock::now() - t0).count() < secs) { + MSG msg; + while (PeekMessageW(&msg, nullptr, 0, 0, PM_REMOVE)) { + TranslateMessage(&msg); + DispatchMessageW(&msg); + } + Sleep(2); + } + // Stay alive 5s with BOTH editors attached and the worker pumping, + // while MAIN KEEPS PUMPING like the bridge audio loop: distinguishes a + // live crash (bad) from a stop-pumping probe artifact. + auto t1 = std::chrono::steady_clock::now(); + while (std::chrono::duration_cast( + std::chrono::steady_clock::now() - t1).count() < 5) { + MSG msg; + while (PeekMessageW(&msg, nullptr, 0, 0, PM_REMOVE)) { + TranslateMessage(&msg); + DispatchMessageW(&msg); + } + Sleep(2); + } + printf("RESULT: done=%d aRes=%d bRes=%d\n", done.load() ? 1 : 0, aRes, bRes); + return (done.load() && aRes && bRes) ? 0 : 3; + } else if (variant == "bridge_two") { + // Exact bridge architecture: per-channel persistent worker threads with + // own COM STA apartments and idle message pumps; inst1 loaded+attached + // on worker A, then inst2 loaded+attached on worker B; main pumps. + ProbeWorker wa, wb; + wa.start(); + wb.start(); + std::atomic aDone{false}, aOk{false}, bDone{false}, bOk{false}; + wa.post([&]() { + printf("[bridge_two] workerA load inst1\n"); + fflush(stdout); + bool r = inst.loadPlugin(path, 44100.0); + printf("[bridge_two] workerA load inst1=%d\n", r ? 1 : 0); + fflush(stdout); + if (r) { + HWND h = CreateWindowEx(0, "GuiProbeClass", "GuiProbeA", WS_OVERLAPPEDWINDOW, + 0, 0, 800, 600, nullptr, nullptr, GetModuleHandle(nullptr), nullptr); + printf("[bridge_two] workerA hwnd=%p\n", (void*)h); + fflush(stdout); + printf("[bridge_two] workerA openGUI inst1\n"); + fflush(stdout); + r = inst.openGUI(h); + printf("[bridge_two] workerA openGUI inst1=%d\n", r ? 1 : 0); + fflush(stdout); + aOk = r; + } + aDone = true; + }); + auto t0 = std::chrono::steady_clock::now(); + auto elapsed = [&]() { + return std::chrono::duration_cast( + std::chrono::steady_clock::now() - t0).count(); + }; + // main pump like the bridge audio loop + std::thread pump([&]() { + while (elapsed() < secs) { + MSG msg; + while (PeekMessageW(&msg, nullptr, 0, 0, PM_REMOVE)) { + TranslateMessage(&msg); + DispatchMessageW(&msg); + } + Sleep(2); + } + }); + while (!aDone.load() && elapsed() < secs) Sleep(50); + if (!aDone.load()) { + printf("RESULT: TIMEOUT inst1 attach (aDone=0)\n"); + return 3; + } + wb.post([&]() { + printf("[bridge_two] workerB load inst2\n"); + fflush(stdout); + bool r = inst2.loadPlugin(path, 44100.0); + printf("[bridge_two] workerB load inst2=%d\n", r ? 1 : 0); + fflush(stdout); + if (r) { + HWND h = CreateWindowEx(0, "GuiProbeClass", "GuiProbeB", WS_OVERLAPPEDWINDOW, + 0, 0, 800, 600, nullptr, nullptr, GetModuleHandle(nullptr), nullptr); + printf("[bridge_two] workerB hwnd=%p\n", (void*)h); + fflush(stdout); + printf("[bridge_two] workerB openGUI inst2\n"); + fflush(stdout); + r = inst2.openGUI(h); + printf("[bridge_two] workerB openGUI inst2=%d\n", r ? 1 : 0); + fflush(stdout); + bOk = r; + } + bDone = true; + }); + while ((!aDone.load() || !bDone.load()) && elapsed() < secs) Sleep(50); + pump.join(); + printf("RESULT: aDone=%d aOk=%d bDone=%d bOk=%d\n", + aDone.load() ? 1 : 0, aOk.load() ? 1 : 0, + bDone.load() ? 1 : 0, bOk.load() ? 1 : 0); + return (aDone.load() && aOk.load() && bDone.load() && bOk.load()) ? 0 : 3; + } else if (variant == "two_instances" || variant == "two_instances_close" || variant == "close_reopen") { + // Bridge round-3 probes: does a SECOND instance of the same plugin hang + // in view->attached() while the first instance's view is attached? + // Does closing the first view first (two_instances_close) or the + // close-then-reopen reload path (close_reopen) avoid the hang? + auto attachOnWorker = [&](Vst3Instrument* i, void* hwnd, const char* tag) -> bool { + std::atomic d{false}; + std::atomic r{false}; + std::thread t([&]() { + CoInitializeEx(nullptr, COINIT_APARTMENTTHREADED); + printf("[%s] openGUI(%s) start hwnd=%p\n", variant.c_str(), tag, hwnd); + fflush(stdout); + bool rr = i->openGUI(hwnd); + printf("[%s] openGUI(%s) returned=%d\n", variant.c_str(), tag, rr ? 1 : 0); + fflush(stdout); + r = rr; + d = true; + }); + auto t0 = std::chrono::steady_clock::now(); + while (!d.load() && std::chrono::duration_cast( + std::chrono::steady_clock::now() - t0).count() < secs) { + MSG msg; + while (PeekMessageW(&msg, nullptr, 0, 0, PM_REMOVE)) { + TranslateMessage(&msg); + DispatchMessageW(&msg); + } + Sleep(2); + } + t.detach(); + if (!d.load()) { + printf("[%s] TIMEOUT openGUI(%s)\n", variant.c_str(), tag); + return false; + } + return r.load(); + }; + auto loadOnMain = [&](Vst3Instrument* i, const char* tag) -> bool { + printf("[%s] loadPlugin(%s) on main\n", variant.c_str(), tag); + fflush(stdout); + bool rr = i->loadPlugin(path, 44100.0); + printf("[%s] load(%s)=%d\n", variant.c_str(), tag, rr ? 1 : 0); + fflush(stdout); + return rr; + }; + // second parent window for inst2 + HWND hwnd2 = CreateWindowEx(0, "GuiProbeClass", "GuiProbe2", WS_OVERLAPPEDWINDOW, + 0, 0, 800, 600, nullptr, nullptr, GetModuleHandle(nullptr), nullptr); + printf("hwnd2=%p\n", (void*)hwnd2); + if (!loadOnMain(&inst, "inst1")) return 1; + if (!attachOnWorker(&inst, targetHwnd, "inst1")) return 3; + if (variant == "two_instances_close") { + printf("[%s] closeGUI(inst1) before inst2 attach\n", variant.c_str()); + inst.closeGUI(); + } + if (variant == "close_reopen") { + printf("[%s] closeGUI(inst1) then reopen same instance (reload path)\n", variant.c_str()); + inst.closeGUI(); + if (!attachOnWorker(&inst, targetHwnd, "inst1-reopen")) { + printf("RESULT: done=%d attached_ok=%d\n", 0, 0); + return 3; + } + printf("RESULT: done=%d attached_ok=%d\n", 1, 1); + return 0; + } + if (!loadOnMain(&inst2, "inst2")) return 1; + if (!attachOnWorker(&inst2, hwnd2, "inst2")) { + printf("RESULT: done=%d attached_ok=%d\n", 0, 0); + return 3; + } + // 5s survival with both editors attached (main keeps pumping). + auto t1 = std::chrono::steady_clock::now(); + while (std::chrono::duration_cast( + std::chrono::steady_clock::now() - t1).count() < 5) { + MSG msg; + while (PeekMessageW(&msg, nullptr, 0, 0, PM_REMOVE)) { + TranslateMessage(&msg); + DispatchMessageW(&msg); + } + Sleep(2); + } + printf("RESULT: done=%d attached_ok=%d\n", 1, 1); + return 0; } else { printf("unknown variant %s\n", variant.c_str()); return 2; diff --git a/native_bridge/src/main.cpp b/native_bridge/src/main.cpp index 72d1485..5a36ff4 100644 --- a/native_bridge/src/main.cpp +++ b/native_bridge/src/main.cpp @@ -27,6 +27,7 @@ #include #include #include +#include // --- platform helpers ------------------------------------------------------- static bool parent_alive(uint32_t pid) { @@ -366,6 +367,48 @@ int main(int argc, char* argv[]) { << " plugin=" << arg2 << " ch=" << guiCh << std::endl; void* hwnd = (void*)(uintptr_t)arg1; #ifdef _WIN32 + // Option B: chi 1 editor VST mo tai 1 thoi diem toan + // bridge. Instance thu 2 cua CUNG plugin (Nexus) attach + // view o apartment/worker khac -> treo (bridge.log: ch=2 + // ket o attached() khi ch=1 con mo) / crash (gui_probe + // bridge_two). Dong editor cua channel khac TRUOC khi + // attach: WM_CLOSE -> worker channel do destroy window -> + // WM_DESTROY -> closeGUI() -> view->removed() tren dung + // thread so huu (gui_probe two_instances_close: OK). + { + std::vector others; + { + std::lock_guard lock(g_guiMutex); + for (const auto& kv : g_guiWindows) + if (kv.first != guiCh) others.push_back(kv.first); + } + for (uint32_t y : others) { + HWND yHwnd = nullptr; + { + std::lock_guard lock(g_guiMutex); + auto it = g_guiWindows.find(y); + if (it != g_guiWindows.end()) yHwnd = (HWND)it->second; + } + if (!yHwnd || !IsWindow(yHwnd)) continue; + PostMessage(yHwnd, WM_CLOSE, 0, 0); + std::cerr << "[dbg] openGUI: closing editor ch=" << y + << " before attach ch=" << guiCh << std::endl; + // Cho worker channel y xu ly WM_CLOSE -> destroy -> + // WM_DESTROY (xoa registry). Timeout 5s tranh + // worker ket (load lau / plugin treo). + bool closed = false; + for (int i = 0; i < 500; ++i) { + { + std::lock_guard lock(g_guiMutex); + if (g_guiWindows.find(y) == g_guiWindows.end()) { closed = true; break; } + } + Sleep(10); + } + if (!closed) + std::cerr << "[dbg] openGUI: editor ch=" << y + << " not closed in 5s, proceeding" << std::endl; + } + } if (arg1 == 0) { HWND existingHwnd = nullptr; { @@ -400,7 +443,17 @@ int main(int argc, char* argv[]) { (void)0; #endif if (auto* inst = instruments.get(guiCh)) { - if (inst->openGUI(hwnd)) + // Reopen sau khi dong: openGUI() rebuild lai plugin + // instance (reload = terminate + loadPlugin) trong + // khi audio loop co the dang processAudioBlock tren + // CUNG object -> UAF/hang. Chan bang flag reloading_ + // (set/clear duoi engine mutex; renderAll giu mutex + // khi process nen check luon nhat quan). + bool guard = inst->needsReload(); + if (guard) instruments.setReloading(guiCh, true); + bool ok = inst->openGUI(hwnd); + if (guard) instruments.setReloading(guiCh, false); + if (ok) std::cout << "[NativeBridge] GUI attached hwnd=" << hwnd << " plugin=" << arg2 << " ch=" << guiCh << std::endl; else