mirror of
https://github.com/got-feedBack/feedBack-desktop.git
synced 2026-09-10 22:14:10 +00:00
fix(audio): header-only module pin; exception-safe ops; CI link fix
CI: slopsmith-vst-host (sandbox-e2e's own CMake project) and macOS's slopsmith-vst-scan compile VSTHost.cpp without LifecycleExecutor.cpp and failed to link pinPluginModuleForever. Move the pin to a header-only PluginModulePin.h so every target that compiles VSTHost.cpp gets it with no extra source-list entry; drop the now-unneeded LifecycleExecutor.cpp from VST_HOST_SOURCES. CodeRabbit review: - Pin by full module path, not base name: enumerate loaded modules (PSAPI_VERSION=2 K32 exports, no psapi.lib) and pin every one whose path starts with the plugin identifier — two plugins sharing a DLL base name can no longer pin the wrong module, and the bundle-dir identifier now matches the inner Contents/<arch>/ file it actually loaded. - Ops are exception-safe: a throwing op logs and still signals its waiter instead of leaving an unbounded caller blocked forever. - Documented why the message-thread inline path is intentional (nested submissions are part of the outer op; dispatch-and-wait on the message thread would self-deadlock; FIFO is the contract between off-thread submitters). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Fable 5
parent
503f665865
commit
4ac1b60766
@@ -1,6 +1,6 @@
|
||||
#include "VSTHost.h"
|
||||
#include "VSTTrace.h"
|
||||
#include "addon/LifecycleExecutor.h"
|
||||
#include "addon/PluginModulePin.h"
|
||||
|
||||
// The out-of-process scan path is compiled only into the audio addon
|
||||
// (SLOPSMITH_AUDIO_ADDON, set in src/audio/CMakeLists.txt). slopsmith-vst-host
|
||||
|
||||
@@ -7,14 +7,10 @@
|
||||
|
||||
#include <atomic>
|
||||
#include <cstdio>
|
||||
#include <exception>
|
||||
#include <memory>
|
||||
#include <mutex>
|
||||
|
||||
#if JUCE_WINDOWS
|
||||
#define WIN32_LEAN_AND_MEAN
|
||||
#include <windows.h>
|
||||
#endif
|
||||
|
||||
namespace slopsmith::addon {
|
||||
|
||||
static std::atomic<std::uint64_t> engineGeneration{1};
|
||||
@@ -50,9 +46,17 @@ LifecycleOpResult runLifecycleOp(const char* name,
|
||||
return LifecycleOpResult::neverRan;
|
||||
}
|
||||
|
||||
// Already on the message thread (engine init path, macOS inline mode):
|
||||
// dispatch-and-wait on ourselves would deadlock. Run inline — we are by
|
||||
// definition serialized with every queued op.
|
||||
// Already on the message thread (engine init path, macOS inline mode,
|
||||
// or a lifecycle op submitting a nested op): dispatch-and-wait on
|
||||
// ourselves would deadlock, so run inline.
|
||||
//
|
||||
// Ordering note (CodeRabbit #120): an inline op runs as part of the
|
||||
// CURRENT message-thread turn, i.e. ahead of ops still queued behind
|
||||
// it. That is intentional, not a FIFO leak: a nested submission is
|
||||
// semantically part of the outer op and must complete inside it, and a
|
||||
// message-thread caller can never interleave with a *running* op — the
|
||||
// queue drains on this very thread. The FIFO contract is between
|
||||
// off-thread submitters, which all take the queued path below.
|
||||
if (mm->isThisTheMessageThread())
|
||||
{
|
||||
if (stamped != currentEngineGeneration())
|
||||
@@ -60,7 +64,13 @@ LifecycleOpResult runLifecycleOp(const char* name,
|
||||
fprintf(stderr, "[lifecycle] op '%s': stale generation (inline); skipped\n", name);
|
||||
return LifecycleOpResult::stale;
|
||||
}
|
||||
func();
|
||||
try { func(); }
|
||||
catch (const std::exception& e) {
|
||||
fprintf(stderr, "[lifecycle] op '%s' (inline) threw: %s\n", name, e.what());
|
||||
}
|
||||
catch (...) {
|
||||
fprintf(stderr, "[lifecycle] op '%s' (inline) threw (unknown)\n", name);
|
||||
}
|
||||
return LifecycleOpResult::completed;
|
||||
}
|
||||
|
||||
@@ -86,7 +96,16 @@ LifecycleOpResult runLifecycleOp(const char* name,
|
||||
}
|
||||
else
|
||||
{
|
||||
func();
|
||||
// Always reach the signal below: a throwing op would
|
||||
// otherwise leave its unbounded caller waiting forever
|
||||
// (CodeRabbit #120).
|
||||
try { func(); }
|
||||
catch (const std::exception& e) {
|
||||
fprintf(stderr, "[lifecycle] op '%s' threw: %s\n", name, e.what());
|
||||
}
|
||||
catch (...) {
|
||||
fprintf(stderr, "[lifecycle] op '%s' threw (unknown)\n", name);
|
||||
}
|
||||
}
|
||||
shared->done.signal();
|
||||
});
|
||||
@@ -133,35 +152,4 @@ LifecycleOpResult runLifecycleOp(const char* name,
|
||||
: LifecycleOpResult::completed;
|
||||
}
|
||||
|
||||
void pinPluginModuleForever(const char* fileOrIdentifierUtf8)
|
||||
{
|
||||
#if JUCE_WINDOWS
|
||||
// JUCE loads the inner <bundle>/Contents/x86_64-win/<name>.vst3 (or a
|
||||
// flat .vst3/.dll); either way the loaded module's base name is the
|
||||
// path's final component. Pin by that name so the loader never unloads
|
||||
// it, even when JUCE's module refcount hits zero.
|
||||
const juce::String path = juce::String::fromUTF8(fileOrIdentifierUtf8);
|
||||
const juce::String base = path.replaceCharacter('\\', '/')
|
||||
.fromLastOccurrenceOf("/", false, false);
|
||||
if (base.isEmpty())
|
||||
return;
|
||||
|
||||
HMODULE h = nullptr;
|
||||
if (GetModuleHandleExW(GET_MODULE_HANDLE_EX_FLAG_PIN,
|
||||
base.toWideCharPointer(), &h))
|
||||
{
|
||||
fprintf(stderr, "[lifecycle] pinned plugin module '%s' for process "
|
||||
"lifetime\n", base.toRawUTF8());
|
||||
}
|
||||
// Not currently loaded under that name (sandboxed plugin, or the inner
|
||||
// file is named differently): nothing to pin — the sandbox child owns
|
||||
// its own modules, and an in-process module we can't resolve here was
|
||||
// loaded under some other base name and will be pinned on a later load
|
||||
// if it ever resolves. Silent by design; this is a belt on JUCE's
|
||||
// refcount braces, not a correctness gate.
|
||||
#else
|
||||
(void) fileOrIdentifierUtf8;
|
||||
#endif
|
||||
}
|
||||
|
||||
} // namespace slopsmith::addon
|
||||
|
||||
@@ -70,14 +70,9 @@ inline bool runLifecycleOpOk(const char* name, std::function<void()> func,
|
||||
== LifecycleOpResult::completed;
|
||||
}
|
||||
|
||||
// Pin a plugin module so the OS never unloads it for the life of the
|
||||
// process (guide §12 P0 item 4). JUCE refcounts VST3 modules and unloads
|
||||
// them when the last instance dies; a window/timer message already queued
|
||||
// to that module's code then fires into unmapped or reused pages — the CFG
|
||||
// fail-fast (0xc0000409 / FAST_FAIL_GUARD_ICALL_CHECK_FAILURE) in the
|
||||
// 2026-07-17 dumps. The pin is by module base name (the inner
|
||||
// x86_64-win/*.vst3 file JUCE actually loads shares the bundle's base
|
||||
// name). No-op off Windows and for modules that are not currently loaded.
|
||||
void pinPluginModuleForever(const char* fileOrIdentifierUtf8);
|
||||
// Plugin-module pinning (guide §12 P0 item 4) lives in PluginModulePin.h —
|
||||
// header-only, because VSTHost.cpp is compiled into targets across three
|
||||
// CMake projects and an out-of-line definition broke the ones that don't
|
||||
// build this executor (PR #120 CI).
|
||||
|
||||
} // namespace slopsmith::addon
|
||||
|
||||
@@ -0,0 +1,99 @@
|
||||
#pragma once
|
||||
|
||||
// Plugin-module pinning — P0 item 4 of the audio architecture guide (§12).
|
||||
//
|
||||
// JUCE refcounts VST3 modules and unloads them when the last instance dies;
|
||||
// a window/timer message already queued to that module's code then fires
|
||||
// into unmapped or reused pages — the CFG fail-fast
|
||||
// (0xc0000409 / FAST_FAIL_GUARD_ICALL_CHECK_FAILURE) in the 2026-07-17
|
||||
// field dumps. Pinning tells the loader to never unload the module for the
|
||||
// life of the process.
|
||||
//
|
||||
// Header-only ON PURPOSE: VSTHost.cpp is compiled into four different
|
||||
// targets across three CMake projects (the addon, slopsmith-vst-host in
|
||||
// two projects, slopsmith-vst-scan on macOS); an out-of-line definition
|
||||
// broke every link that didn't add the extra .cpp (PR #120 CI).
|
||||
//
|
||||
// The pin matches loaded modules by PATH, not base name: the identifier we
|
||||
// get is usually the bundle directory (…\Foo.vst3\), while the loader knows
|
||||
// the inner …\Contents\x86_64-win\Foo.vst3 file — and two plugins may share
|
||||
// a base name. Enumerate loaded modules and pin every one whose full path
|
||||
// starts with the normalized identifier (K32* exports so no psapi.lib).
|
||||
// No-op off Windows and for modules that are not currently loaded (e.g.
|
||||
// sandboxed plugins — the child process owns those).
|
||||
|
||||
#include <cstdio>
|
||||
|
||||
#if defined(_WIN32)
|
||||
#ifndef WIN32_LEAN_AND_MEAN
|
||||
#define WIN32_LEAN_AND_MEAN
|
||||
#endif
|
||||
#ifndef NOMINMAX
|
||||
#define NOMINMAX
|
||||
#endif
|
||||
#include <windows.h>
|
||||
// PSAPI_VERSION 2 maps EnumProcessModules/GetModuleFileNameExW onto the
|
||||
// kernel32-exported K32* variants — no psapi.lib link needed (matters for
|
||||
// the extra targets this header serves; see the header-only note above).
|
||||
#ifndef PSAPI_VERSION
|
||||
#define PSAPI_VERSION 2
|
||||
#endif
|
||||
#include <psapi.h>
|
||||
#include <vector>
|
||||
#include <cwctype>
|
||||
#endif
|
||||
|
||||
#include <juce_core/juce_core.h>
|
||||
|
||||
namespace slopsmith::addon {
|
||||
|
||||
inline void pinPluginModuleForever(const char* fileOrIdentifierUtf8)
|
||||
{
|
||||
#if defined(_WIN32)
|
||||
const juce::String wanted =
|
||||
juce::String::fromUTF8(fileOrIdentifierUtf8).replaceCharacter('/', '\\');
|
||||
if (wanted.isEmpty())
|
||||
return;
|
||||
|
||||
auto startsWithIgnoreCase = [](const wchar_t* full, const wchar_t* prefix) {
|
||||
while (*prefix != 0)
|
||||
{
|
||||
if (*full == 0
|
||||
|| std::towlower(static_cast<wint_t>(*full))
|
||||
!= std::towlower(static_cast<wint_t>(*prefix)))
|
||||
return false;
|
||||
++full;
|
||||
++prefix;
|
||||
}
|
||||
return true;
|
||||
};
|
||||
|
||||
std::vector<HMODULE> modules(1024);
|
||||
DWORD needed = 0;
|
||||
if (!EnumProcessModules(GetCurrentProcess(), modules.data(),
|
||||
static_cast<DWORD>(modules.size() * sizeof(HMODULE)),
|
||||
&needed))
|
||||
return;
|
||||
modules.resize(needed / sizeof(HMODULE));
|
||||
|
||||
for (HMODULE mod : modules)
|
||||
{
|
||||
wchar_t modPath[MAX_PATH * 2] = {};
|
||||
if (GetModuleFileNameExW(GetCurrentProcess(), mod, modPath,
|
||||
static_cast<DWORD>(sizeof(modPath) / sizeof(modPath[0]))) == 0)
|
||||
continue;
|
||||
if (!startsWithIgnoreCase(modPath, wanted.toWideCharPointer()))
|
||||
continue;
|
||||
|
||||
HMODULE pinned = nullptr;
|
||||
if (GetModuleHandleExW(GET_MODULE_HANDLE_EX_FLAG_PIN, modPath, &pinned))
|
||||
fprintf(stderr, "[lifecycle] pinned plugin module '%s' for process "
|
||||
"lifetime\n",
|
||||
juce::String(modPath).toRawUTF8());
|
||||
}
|
||||
#else
|
||||
(void) fileOrIdentifierUtf8;
|
||||
#endif
|
||||
}
|
||||
|
||||
} // namespace slopsmith::addon
|
||||
@@ -63,7 +63,6 @@ endfunction()
|
||||
set(VST_HOST_SOURCES
|
||||
main.cpp
|
||||
../audio/VSTHost.cpp
|
||||
../audio/addon/LifecycleExecutor.cpp
|
||||
../audio/Sandbox/Protocol.cpp
|
||||
../audio/Sandbox/ControlChannel_shared.cpp
|
||||
../audio/Sandbox/AudioChannel_shared.cpp
|
||||
|
||||
Reference in New Issue
Block a user