From c05a5017e2f8e511f503fb12b0fffb1e55da8495 Mon Sep 17 00:00:00 2001 From: Stefan Kersten Date: Mon, 1 Jun 2026 23:35:30 +0200 Subject: [PATCH 1/5] Add generic RT/NRT shared resource system (#147) Introduces Methcla_ResourceDef plugin API for defining named resource types, OSC commands /resource/new and /resource/free, and /resource/ready, /resource/error, /resource/destroyed notifications. Supports freePending to handle races between in-flight construction and a concurrent free request. AsyncLatch condvar helper replaces sleepFor() in tests. --- CHANGELOG.md | 1 + include/methcla/engine.hpp | 59 +++++++++++ include/methcla/plugin.h | 50 +++++++++ src/Methcla/API.cpp | 2 + src/Methcla/Audio/Engine.cpp | 16 ++- src/Methcla/Audio/Engine.hpp | 4 + src/Methcla/Audio/EngineImpl.cpp | 148 ++++++++++++++++++++++++++ src/Methcla/Audio/EngineImpl.hpp | 111 +++++++++++++++++++ tests/CMakeLists.txt | 9 +- tests/methcla_tests.hpp | 25 +++++ tests/resource_tests.cpp | 177 +++++++++++++++++++++++++++++++ tests/test_resource.hpp | 46 ++++++++ 12 files changed, 645 insertions(+), 3 deletions(-) create mode 100644 tests/resource_tests.cpp create mode 100644 tests/test_resource.hpp diff --git a/CHANGELOG.md b/CHANGELOG.md index b0e8c975..d7c8c839 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - `METHCLA_BUILD_TESTS` CMake option, defaults to on when building as top-level project (#122) - CMake presets (`debug`, `release`) and GitHub Actions CI replacing ad-hoc configuration and Travis CI (#118) - Project documentation: CONTRIBUTING guide, architecture overview with Mermaid diagram, OSC API reference, examples README +- Generic RT/NRT shared resource system (#147): `Methcla_ResourceDef` plugin API, `ResourceId` and `allocResourceId`/`freeResourceId` client API, OSC commands `/resource/new` and `/resource/free`, notifications `/resource/ready`, `/resource/error`, `/resource/destroyed`, with `freePending` support for races between construction and free ### Changed diff --git a/include/methcla/engine.hpp b/include/methcla/engine.hpp index e3eedffc..46934bd7 100644 --- a/include/methcla/engine.hpp +++ b/include/methcla/engine.hpp @@ -86,6 +86,29 @@ namespace Methcla { {} }; + class ResourceId : public detail::Id + { + public: + explicit ResourceId(int32_t id) + : Id(id) + {} + ResourceId() + : ResourceId(-1) + {} + + operator bool() const + { + return *this != ResourceId(); + } + }; + + inline static std::ostream& operator<<(std::ostream& out, + const ResourceId& id) + { + out << id.id(); + return out; + } + // Node placement specification given a target. class NodePlacement { @@ -509,6 +532,7 @@ namespace Methcla { size_t realtimeMemorySize = 1024 * 1024; size_t maxNumNodes = 1024; size_t maxNumAudioBuses = 128; + size_t maxNumResources = 256; size_t maxNumControlBuses = 4096; size_t sampleRate = 44100; size_t blockSize = 64; @@ -590,6 +614,7 @@ namespace Methcla { typedef IdAllocator NodeIdAllocator; typedef IdAllocator AudioBusIdAllocator; + typedef IdAllocator ResourceIdAllocator; class Request; @@ -863,6 +888,28 @@ namespace Methcla { .int32(flags) .closeMessage(); } + + void resourceNew(ResourceId id, const char* uri) + { + beginMessage(); + + oscPacket() + .openMessage("/resource/new", 3) + .int32(0) // requestId (unused) + .int32(id.id()) + .string(uri) + .closeMessage(); + } + + void resourceFree(ResourceId id) + { + beginMessage(); + + oscPacket() + .openMessage("/resource/free", 1) + .int32(id.id()) + .closeMessage(); + } }; void EngineInterface::bundle(Methcla_Time time, @@ -944,6 +991,7 @@ namespace Methcla { : m_logHandler(inOptions.logHandler) , m_nodeIds(1, inOptions.maxNumNodes - 1) , m_audioBusIds(0, inOptions.maxNumAudioBuses) + , m_resourceIds(0, inOptions.maxNumResources) , m_requestId(kMethcla_Notification + 1) , m_notificationHandlerId(0) , m_packets(8192) @@ -1028,6 +1076,16 @@ namespace Methcla { return m_audioBusIds; } + ResourceId allocResourceId() + { + return m_resourceIds.alloc(); + } + + void freeResourceId(ResourceId id) + { + m_resourceIds.free(id); + } + std::unique_ptr allocPacket() override { return std::make_unique(m_packets); @@ -1311,6 +1369,7 @@ namespace Methcla { LogHandler m_logHandler; NodeIdAllocator m_nodeIds; AudioBusIdAllocator m_audioBusIds; + ResourceIdAllocator m_resourceIds; Methcla_RequestId m_requestId; std::mutex m_requestIdMutex; ResponseHandlers m_responseHandlers; diff --git a/include/methcla/plugin.h b/include/methcla/plugin.h index 2a5c369a..4ce36216 100644 --- a/include/methcla/plugin.h +++ b/include/methcla/plugin.h @@ -12,6 +12,7 @@ #include #include #include +#include #if defined(__cplusplus) extern "C" { @@ -146,6 +147,42 @@ static inline void methcla_world_synth_done(Methcla_World* world, world->synth_done(world, synth); } +typedef int32_t Methcla_ResourceId; + +typedef enum +{ + kMethcla_Immutable, + kMethcla_Mutable +} Methcla_ResourceMutability; + +typedef struct Methcla_ResourceDef Methcla_ResourceDef; + +struct Methcla_ResourceDef +{ + //* Unique resource type URI. + const char* uri; + + //* Size of an instance in bytes. + size_t instance_size; + + //* Size of options struct in bytes. + size_t options_size; + + //* Mutability hint. + Methcla_ResourceMutability mutability; + + //* Parse OSC options and fill options struct. + void (*configure)(const void* tag_buffer, size_t tag_size, + const void* arg_buffer, size_t arg_size, void* options); + + //* Construct a resource instance at the given location. + void (*construct)(Methcla_Host* host, const Methcla_ResourceDef* def, + const void* options, void* instance); + + //* Destroy a resource instance. + void (*destroy)(Methcla_Host* host, void* instance); +}; + typedef enum { kMethcla_Input, @@ -262,8 +299,21 @@ struct Methcla_Host //* Log a message and a newline character. void (*log_line)(Methcla_Host* host, Methcla_LogLevel level, const char* message); + + //* Register a resource type definition. + void (*register_resource_def)(Methcla_Host* host, + const Methcla_ResourceDef* def); }; +static inline void +methcla_host_register_resource_def(Methcla_Host* host, + const Methcla_ResourceDef* def) +{ + assert(host && host->register_resource_def); + assert(def); + host->register_resource_def(host, def); +} + static inline void methcla_host_register_synthdef(Methcla_Host* host, const Methcla_SynthDef* synthDef) diff --git a/src/Methcla/API.cpp b/src/Methcla/API.cpp index f9f3b955..52a71865 100644 --- a/src/Methcla/API.cpp +++ b/src/Methcla/API.cpp @@ -94,6 +94,7 @@ struct Methcla_EngineOptions size_t realtime_memory_size = 1024 * 1024; size_t max_num_nodes = 1024; size_t max_num_audio_buses = 1024; + size_t max_num_resources = 256; Methcla_LogLevel log_level = kMethcla_LogWarn; @@ -114,6 +115,7 @@ Methcla::API::convertOptions(const Methcla_EngineOptions* options) result.realtimeMemorySize = options->realtime_memory_size; result.maxNumNodes = options->max_num_nodes; result.maxNumAudioBuses = options->max_num_audio_buses; + result.maxNumResources = options->max_num_resources; if (options->plugin_libraries != nullptr) { diff --git a/src/Methcla/Audio/Engine.cpp b/src/Methcla/Audio/Engine.cpp index 0d84caf1..f36a8316 100644 --- a/src/Methcla/Audio/Engine.cpp +++ b/src/Methcla/Audio/Engine.cpp @@ -49,6 +49,15 @@ namespace { static_cast(host->handle)->registerSynthDef(synthDef); } + METHCLA_C_LINKAGE void + methcla_api_host_register_resource_def(Methcla_Host* host, + const Methcla_ResourceDef* def) + { + assert(host && host->handle); + assert(def); + static_cast(host->handle)->registerResourceDef(def); + } + METHCLA_C_LINKAGE void methcla_api_host_register_soundfile_api(Methcla_Host* host, Methcla_SoundFileAPI* api) @@ -295,7 +304,7 @@ Environment::Environment(LogHandler logHandler, PacketHandler packetHandler, methcla_api_host_free, methcla_api_host_alloc_aligned, methcla_api_host_free_aligned, methcla_api_host_soundfile_open, methcla_api_host_perform_command, methcla_api_host_notify, - methcla_api_host_log_line}) + methcla_api_host_log_line, methcla_api_host_register_resource_def}) // Methcla_World interface , m_world({this, methcla_api_world_samplerate, methcla_api_world_block_size, @@ -467,6 +476,11 @@ void Environment::registerSynthDef(const Methcla_SynthDef* def) m_impl->registerSynthDef(def); } +void Environment::registerResourceDef(const Methcla_ResourceDef* def) +{ + m_impl->registerResourceDef(def); +} + const std::shared_ptr& Environment::synthDef(const char* uri) const { return m_impl->synthDef(uri); diff --git a/src/Methcla/Audio/Engine.hpp b/src/Methcla/Audio/Engine.hpp index 51279680..405a2982 100644 --- a/src/Methcla/Audio/Engine.hpp +++ b/src/Methcla/Audio/Engine.hpp @@ -55,6 +55,7 @@ namespace Methcla { namespace Audio { size_t realtimeMemorySize = 1024 * 1024; size_t maxNumNodes = 1024; size_t maxNumAudioBuses = 1024; + size_t maxNumResources = 256; size_t maxNumControlBuses = 4096; size_t sampleRate = 44100; size_t blockSize = 64; @@ -138,6 +139,9 @@ namespace Methcla { namespace Audio { //* Register SynthDef. void registerSynthDef(const Methcla_SynthDef* synthDef); + //* Register ResourceDef. + void registerResourceDef(const Methcla_ResourceDef* def); + //* Lookup SynthDef const std::shared_ptr& synthDef(const char* uri) const; diff --git a/src/Methcla/Audio/EngineImpl.cpp b/src/Methcla/Audio/EngineImpl.cpp index a62bed87..9e898c50 100644 --- a/src/Methcla/Audio/EngineImpl.cpp +++ b/src/Methcla/Audio/EngineImpl.cpp @@ -214,6 +214,7 @@ EnvironmentImpl::EnvironmentImpl(Environment* owner, LogHandler logHandler, , m_epoch(0) , m_currentTime(0) , m_nodes(options.maxNumNodes, nullptr) +, m_resources(options.maxNumResources) , m_logLevel(options.logLevel) , m_logFlags(kMethcla_EngineLogDefault) { @@ -713,6 +714,107 @@ void EnvironmentImpl::processMessage(Methcla_EngineLogFlags logFlags, sendToWorker(requestId, stats); } + else if (msg == "/resource/new") + { + class ResourceConstructCommand + { + EnvironmentImpl* m_impl; + int32_t m_resourceId; + const Methcla_ResourceDef* m_def; + void* m_options; + void* m_result; + + static void completeOnRT(Environment* env, void* data) + { + auto* self = static_cast(data); + auto& entry = self->m_impl->m_resources[self->m_resourceId]; + entry.def = self->m_def; + entry.data = self->m_result; + + if (entry.freePending) + { + entry.state = ResourceEntry::State::Destroying; + self->m_impl->scheduleResourceDestroy( + self->m_resourceId); + } + else + { + entry.state = ResourceEntry::State::Live; + self->m_impl->notifyResourceReady(self->m_resourceId); + } + env->rtMem().free(self); + } + + public: + ResourceConstructCommand(EnvironmentImpl* impl, + int32_t resourceId, + const Methcla_ResourceDef* def, + void* options) + : m_impl(impl) + , m_resourceId(resourceId) + , m_def(def) + , m_options(options) + , m_result(nullptr) + {} + + void perform(Environment* env) + { + Methcla_Host host(*env); + m_result = Memory::alloc(m_def->instance_size); + if (m_def->construct) + m_def->construct(&host, m_def, m_options, m_result); + if (m_options) + env->sendFromWorker(perform_rt_free, m_options); + env->sendFromWorker(completeOnRT, this); + } + }; + + /* args: requestId:i resourceId:i uri:s [options...] */ + args.int32(); // requestId — unused for now + const int32_t resourceId = args.int32(); + const char* uri = args.string(); + + auto it = m_resourceDefs.find(uri); + if (it == m_resourceDefs.end()) + { + notifyResourceError(resourceId, "Unknown resource type"); + } + else + { + const Methcla_ResourceDef* def = it->second; + auto& entry = m_resources[resourceId]; + entry.state = ResourceEntry::State::Constructing; + entry.freePending = false; + entry.def = nullptr; + entry.data = nullptr; + entry.refCount = 0; + + void* options = nullptr; + if (def->options_size > 0) + options = rtMem().alloc(def->options_size); + + sendToWorker(this, resourceId, def, + options); + } + } + else if (msg == "/resource/free") + { + const int32_t resourceId = args.int32(); + if (resourceId < 0 || + static_cast(resourceId) >= m_resources.size()) + return; + auto& entry = m_resources[resourceId]; + if (entry.state == ResourceEntry::State::Constructing) + { + entry.freePending = true; + } + else if (entry.state == ResourceEntry::State::Live && + entry.refCount == 0) + { + entry.state = ResourceEntry::State::Destroying; + scheduleResourceDestroy(resourceId); + } + } else if (msg == "/engine/realtime-memory/statistics") { class CommandRealtimeMemoryStatistics @@ -764,6 +866,52 @@ void EnvironmentImpl::registerSynthDef(const Methcla_SynthDef* def) m_synthDefs[synthDef->uri()] = synthDef; } +void EnvironmentImpl::registerResourceDef(const Methcla_ResourceDef* def) +{ + m_resourceDefs[def->uri] = def; +} + +namespace { + struct ResourceDestroyCommand + { + EnvironmentImpl* impl; + int32_t resourceId; + const Methcla_ResourceDef* def; + void* data; + + static void completeOnRT(Environment* env, void* d) + { + auto* self = static_cast(d); + auto& entry = self->impl->m_resources[self->resourceId]; + entry.state = EnvironmentImpl::ResourceEntry::State::Free; + entry.def = nullptr; + entry.data = nullptr; + self->impl + ->sendToWorker( + self->resourceId); + env->rtMem().free(self); + } + + void perform(Environment* env) + { + Methcla_Host host(*env); + if (def->destroy) + def->destroy(&host, data); + Memory::free(data); + env->sendFromWorker(completeOnRT, this); + } + }; +} // namespace + +void EnvironmentImpl::scheduleResourceDestroy(int32_t resourceId) +{ + auto& entry = m_resources[resourceId]; + sendToWorker( + perform_perform, + rtMem().construct( + ResourceDestroyCommand{this, resourceId, entry.def, entry.data})); +} + const std::shared_ptr& EnvironmentImpl::synthDef(const char* uri) const { diff --git a/src/Methcla/Audio/EngineImpl.hpp b/src/Methcla/Audio/EngineImpl.hpp index 2a7c90a2..6318c717 100644 --- a/src/Methcla/Audio/EngineImpl.hpp +++ b/src/Methcla/Audio/EngineImpl.hpp @@ -24,6 +24,8 @@ #include #include #include +#include +#include #include // OSC request with reference counting. @@ -261,6 +263,27 @@ namespace Methcla { namespace Audio { SynthDefMap m_synthDefs; std::list m_soundFileAPIs; + using ResourceDefMap = + std::unordered_map; + ResourceDefMap m_resourceDefs; + + struct ResourceEntry + { + enum class State + { + Free, + Constructing, + Live, + Destroying + }; + State state = State::Free; + bool freePending = false; + const Methcla_ResourceDef* def = nullptr; + void* data = nullptr; + size_t refCount = 0; + }; + std::vector m_resources; + std::atomic m_logLevel; std::atomic m_logFlags; @@ -436,6 +459,94 @@ namespace Methcla { namespace Audio { } } + class ResourceErrorNotification : public Notification + { + int32_t m_resourceId; + const char* m_message; // must be a string literal or static string + + public: + ResourceErrorNotification(int32_t resourceId, const char* message) + : m_resourceId(resourceId) + , m_message(message) + {} + + private: + void notify(Environment* env) override + { + static const char* address = "/resource/error"; + OSCPP::Client::DynamicPacket packet( + OSCPP::Size::message(address, 2) + OSCPP::Size::int32(1) + + OSCPP::Size::string(m_message)); + packet.openMessage(address, 2); + packet.int32(m_resourceId); + packet.string(m_message); + packet.closeMessage(); + env->notify(packet); + } + }; + + //* Context: RT + void notifyResourceError(int32_t resourceId, const char* message) + { + sendToWorker(resourceId, message); + } + + class ResourceReadyNotification : public Notification + { + int32_t m_resourceId; + + public: + explicit ResourceReadyNotification(int32_t resourceId) + : m_resourceId(resourceId) + {} + + private: + void notify(Environment* env) override + { + static const char* address = "/resource/ready"; + OSCPP::Client::DynamicPacket packet( + OSCPP::Size::message(address, 1) + OSCPP::Size::int32(1)); + packet.openMessage(address, 1); + packet.int32(m_resourceId); + packet.closeMessage(); + env->notify(packet); + } + }; + + //* Context: RT + void notifyResourceReady(int32_t resourceId) + { + sendToWorker(resourceId); + } + + void registerResourceDef(const Methcla_ResourceDef* def); + + //* Context: RT — schedule NRT destroy + /resource/destroyed + // notification. + void scheduleResourceDestroy(int32_t resourceId); + + class ResourceDestroyedNotification : public Notification + { + int32_t m_resourceId; + + public: + explicit ResourceDestroyedNotification(int32_t resourceId) + : m_resourceId(resourceId) + {} + + private: + void notify(Environment* env) override + { + static const char* address = "/resource/destroyed"; + OSCPP::Client::DynamicPacket packet( + OSCPP::Size::message(address, 1) + OSCPP::Size::int32(1)); + packet.openMessage(address, 1); + packet.int32(m_resourceId); + packet.closeMessage(); + env->notify(packet); + } + }; + //* Context: NRT void reply(Methcla_RequestId requestId, const void* packet, size_t size) { diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index dc365222..0816d093 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -17,8 +17,13 @@ if(METHCLA_BUILD_TESTS) add_executable( methcla_tests - methcla_tests.cpp semaphore_tests.cpp worker_tests.cpp memory_tests.cpp - engine_tests.cpp ${CMAKE_CURRENT_BINARY_DIR}/config.cpp) + methcla_tests.cpp + semaphore_tests.cpp + worker_tests.cpp + memory_tests.cpp + engine_tests.cpp + resource_tests.cpp + ${CMAKE_CURRENT_BINARY_DIR}/config.cpp) target_include_directories(methcla_tests PRIVATE . ../src) target_include_directories(methcla_tests SYSTEM diff --git a/tests/methcla_tests.hpp b/tests/methcla_tests.hpp index 407817c8..bbd02c89 100644 --- a/tests/methcla_tests.hpp +++ b/tests/methcla_tests.hpp @@ -5,9 +5,12 @@ #pragma once +#include #include +#include #include #include +#include #include #include #include @@ -102,4 +105,26 @@ namespace Methcla { namespace Tests { { std::this_thread::sleep_for(std::chrono::duration(seconds)); } + + class AsyncLatch + { + std::mutex m_mtx; + std::condition_variable m_cv; + bool m_signalled = false; + + public: + void signal() + { + std::unique_lock lock(m_mtx); + m_signalled = true; + m_cv.notify_all(); + } + + bool + wait(std::chrono::milliseconds timeout = std::chrono::milliseconds(500)) + { + std::unique_lock lock(m_mtx); + return m_cv.wait_for(lock, timeout, [this] { return m_signalled; }); + } + }; }} // namespace Methcla::Tests diff --git a/tests/resource_tests.cpp b/tests/resource_tests.cpp new file mode 100644 index 00000000..ba39dfc4 --- /dev/null +++ b/tests/resource_tests.cpp @@ -0,0 +1,177 @@ +// Copyright (C) 2026 Methcla contributors +// +// SPDX-License-Identifier: Apache-2.0 + +#include "methcla_tests.hpp" +#include "test_resource.hpp" + +#include +#include + +#include "gtest/gtest.h" + +using namespace Methcla::Tests; + +// Helper: allocate resource, wait for /resource/ready, return latch-signalled +// id. +static void awaitReady(Methcla::Engine& engine, Methcla::ResourceId id, + const char* uri) +{ + AsyncLatch latch; + engine.addNotificationHandler([&, id](const OSCPP::Server::Message& msg) { + if (msg == "/resource/ready" && msg.args().int32() == id.id()) + { + latch.signal(); + return true; + } + return false; + }); + Methcla::Request req(engine); + req.openBundle(); + req.resourceNew(id, uri); + req.closeBundle(); + req.send(); + ASSERT_TRUE(latch.wait()) << "Timed out waiting for /resource/ready"; +} + +// --------------------------------------------------------------------------- +// Slice 1: Unknown URI → /resource/error +// --------------------------------------------------------------------------- + +TEST(ResourceTests, UnknownUriReturnsError) +{ + auto engine = std::make_unique(); + engine->start(); + + AsyncLatch latch; + Methcla::ResourceId id = engine->allocResourceId(); + + engine->addNotificationHandler([&, id](const OSCPP::Server::Message& msg) { + if (msg == "/resource/error" && msg.args().int32() == id.id()) + { + latch.signal(); + return true; + } + return false; + }); + + Methcla::Request req(*engine); + req.openBundle(); + req.resourceNew(id, "methcla://plugins/unknown"); + req.closeBundle(); + req.send(); + + EXPECT_TRUE(latch.wait()); + engine->freeResourceId(id); + engine->stop(); +} + +// --------------------------------------------------------------------------- +// Slice 2: Known URI → /resource/ready +// --------------------------------------------------------------------------- + +TEST(ResourceTests, KnownUriConstructsAndNotifiesReady) +{ + Methcla::EngineOptions opts; + opts.addLibrary(methcla_plugins_test_resource); + auto engine = std::make_unique(opts); + engine->start(); + + Methcla::ResourceId id = engine->allocResourceId(); + awaitReady(*engine, id, METHCLA_TEST_RESOURCE_URI); + + engine->freeResourceId(id); + engine->stop(); +} + +// --------------------------------------------------------------------------- +// Slice 3: Free live resource → /resource/destroyed +// --------------------------------------------------------------------------- + +TEST(ResourceTests, FreeLiveResourceDestroysIt) +{ + Methcla::EngineOptions opts; + opts.addLibrary(methcla_plugins_test_resource); + auto engine = std::make_unique(opts); + engine->start(); + + Methcla::ResourceId id = engine->allocResourceId(); + awaitReady(*engine, id, METHCLA_TEST_RESOURCE_URI); + + AsyncLatch destroyed; + engine->addNotificationHandler([&, id](const OSCPP::Server::Message& msg) { + if (msg == "/resource/destroyed" && msg.args().int32() == id.id()) + { + destroyed.signal(); + return true; + } + return false; + }); + + { + Methcla::Request req(*engine); + req.openBundle(); + req.resourceFree(id); + req.closeBundle(); + req.send(); + } + EXPECT_TRUE(destroyed.wait()); + engine->freeResourceId(id); + engine->stop(); +} + +// --------------------------------------------------------------------------- +// Slice 4: Free during construction → /resource/destroyed (no /resource/ready) +// --------------------------------------------------------------------------- + +TEST(ResourceTests, FreeDuringConstructSetsFreePending) +{ + Methcla::EngineOptions opts; + opts.addLibrary(methcla_plugins_test_resource); + auto engine = std::make_unique(opts); + engine->start(); + + Methcla::ResourceId id = engine->allocResourceId(); + + AsyncLatch ready; + AsyncLatch destroyed; + + engine->addNotificationHandler([&, id](const OSCPP::Server::Message& msg) { + if (msg == "/resource/ready" && msg.args().int32() == id.id()) + { + ready.signal(); + return true; + } + return false; + }); + engine->addNotificationHandler([&, id](const OSCPP::Server::Message& msg) { + if (msg == "/resource/destroyed" && msg.args().int32() == id.id()) + { + destroyed.signal(); + return true; + } + return false; + }); + + // Send /resource/new and /resource/free back-to-back without waiting. + { + Methcla::Request req(*engine); + req.openBundle(); + req.resourceNew(id, METHCLA_TEST_RESOURCE_URI); + req.closeBundle(); + req.send(); + } + { + Methcla::Request req(*engine); + req.openBundle(); + req.resourceFree(id); + req.closeBundle(); + req.send(); + } + + EXPECT_TRUE(destroyed.wait(std::chrono::milliseconds(1000))); + // /resource/ready must NOT have been emitted. + EXPECT_FALSE(ready.wait(std::chrono::milliseconds(0))); + engine->freeResourceId(id); + engine->stop(); +} diff --git a/tests/test_resource.hpp b/tests/test_resource.hpp new file mode 100644 index 00000000..9e98c304 --- /dev/null +++ b/tests/test_resource.hpp @@ -0,0 +1,46 @@ +// Copyright (C) 2026 Methcla contributors +// +// SPDX-License-Identifier: Apache-2.0 + +#pragma once + +#include + +#include + +#define METHCLA_TEST_RESOURCE_URI METHCLA_PLUGINS_URI "/test-resource" + +namespace TestResourcePlugin { + + struct TestResource + { + int dummy = 0; + }; + + static void construct(Methcla_Host*, const Methcla_ResourceDef*, + const void*, void* instance) + { + new (instance) TestResource(); + } + + static void destroy(Methcla_Host*, void* instance) + { + static_cast(instance)->~TestResource(); + } + + static const Methcla_ResourceDef def = {METHCLA_TEST_RESOURCE_URI, + sizeof(TestResource), + 0, + kMethcla_Immutable, + nullptr, + construct, + destroy}; + +} // namespace TestResourcePlugin + +static Methcla_Library* methcla_plugins_test_resource(Methcla_Host* host, + const char*) +{ + methcla_host_register_resource_def(host, &TestResourcePlugin::def); + return nullptr; +} From 038c4595051b4acef924f01c52079d8d42c48724 Mon Sep 17 00:00:00 2001 From: Stefan Kersten Date: Tue, 2 Jun 2026 22:29:24 +0200 Subject: [PATCH 2/5] Clean up resource system API and error handling MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Add kMethcla_UnsupportedResourceTypeError; replace notifyResourceError with throwErrorWith so unknown-URI errors flow through the standard catch/log path instead of a bespoke /resource/error notification - Report out-of-range resource id in /resource/free as ArgumentError - Move ResourceReadyNotification, NodeTreeStatisticsCommand, and RTMemoryStatisticsCommand out of method bodies into the anonymous namespace; rename Command* → *Command for consistent naming - Consolidate operator bool() and operator<< into the IdBase template - Add resourceIdAllocator() to the Engine interface; rename audioBusId() → audioBusIdAllocator() for consistency - Fix Methcla_Host initializer order to match struct field order after register_resource_def was repositioned --- include/methcla/common.h | 3 + include/methcla/detail.hpp | 12 + include/methcla/engine.hpp | 43 +-- include/methcla/plugin.h | 28 +- src/Methcla/Audio/Engine.cpp | 9 +- src/Methcla/Audio/Engine.hpp | 2 +- src/Methcla/Audio/EngineImpl.cpp | 460 +++++++++++++++++-------------- src/Methcla/Audio/EngineImpl.hpp | 90 +----- tests/disksampler_tests.cpp | 4 +- tests/resource_tests.cpp | 53 ++-- 10 files changed, 320 insertions(+), 384 deletions(-) diff --git a/include/methcla/common.h b/include/methcla/common.h index ef34c501..68daace2 100644 --- a/include/methcla/common.h +++ b/include/methcla/common.h @@ -68,6 +68,7 @@ typedef enum kMethcla_SynthDefNotFoundError = 1000, kMethcla_NodeIdError, kMethcla_NodeTypeError, + kMethcla_UnsupportedResourceTypeError, /* File errors */ kMethcla_FileNotFoundError = 2000, @@ -109,6 +110,8 @@ static inline const char* methcla_error_code_description(Methcla_ErrorCode code) return "Invalid node id"; case kMethcla_NodeTypeError: return "Invalid node type"; + case kMethcla_UnsupportedResourceTypeError: + return "Unsupported resource type"; /* File errors */ case kMethcla_FileNotFoundError: diff --git a/include/methcla/detail.hpp b/include/methcla/detail.hpp index bb95dd45..1238683a 100644 --- a/include/methcla/detail.hpp +++ b/include/methcla/detail.hpp @@ -6,6 +6,7 @@ #pragma once #include +#include #include #include @@ -35,6 +36,17 @@ namespace Methcla { namespace detail { return m_id != other.m_id; } + operator bool() const + { + return m_id != static_cast(-1); + } + + friend std::ostream& operator<<(std::ostream& out, const D& id) + { + out << id.m_id; + return out; + } + private: T m_id; }; diff --git a/include/methcla/engine.hpp b/include/methcla/engine.hpp index 46934bd7..23ce49fb 100644 --- a/include/methcla/engine.hpp +++ b/include/methcla/engine.hpp @@ -49,20 +49,8 @@ namespace Methcla { NodeId() : NodeId(-1) {} - - operator bool() const - { - return *this != NodeId(); - } }; - inline static std::ostream& operator<<(std::ostream& out, - const NodeId& nodeId) - { - out << nodeId.id(); - return out; - } - class GroupId : public NodeId { public: @@ -95,20 +83,8 @@ namespace Methcla { ResourceId() : ResourceId(-1) {} - - operator bool() const - { - return *this != ResourceId(); - } }; - inline static std::ostream& operator<<(std::ostream& out, - const ResourceId& id) - { - out << id.id(); - return out; - } - // Node placement specification given a target. class NodePlacement { @@ -629,7 +605,8 @@ namespace Methcla { return GroupId(0); } - virtual NodeIdAllocator& nodeIdAllocator() = 0; + virtual NodeIdAllocator& nodeIdAllocator() = 0; + virtual ResourceIdAllocator& resourceIdAllocator() = 0; virtual std::unique_ptr allocPacket() = 0; virtual void sendPacket(const std::unique_ptr& packet) = 0; @@ -892,10 +869,8 @@ namespace Methcla { void resourceNew(ResourceId id, const char* uri) { beginMessage(); - oscPacket() - .openMessage("/resource/new", 3) - .int32(0) // requestId (unused) + .openMessage("/resource/new", 2) .int32(id.id()) .string(uri) .closeMessage(); @@ -904,7 +879,6 @@ namespace Methcla { void resourceFree(ResourceId id) { beginMessage(); - oscPacket() .openMessage("/resource/free", 1) .int32(id.id()) @@ -1071,19 +1045,14 @@ namespace Methcla { return m_nodeIds; } - AudioBusIdAllocator& audioBusId() + AudioBusIdAllocator& audioBusIdAllocator() { return m_audioBusIds; } - ResourceId allocResourceId() - { - return m_resourceIds.alloc(); - } - - void freeResourceId(ResourceId id) + ResourceIdAllocator& resourceIdAllocator() override { - m_resourceIds.free(id); + return m_resourceIds; } std::unique_ptr allocPacket() override diff --git a/include/methcla/plugin.h b/include/methcla/plugin.h index 4ce36216..8adbaddc 100644 --- a/include/methcla/plugin.h +++ b/include/methcla/plugin.h @@ -262,8 +262,11 @@ struct Methcla_Host void* handle; //* Register a synth definition. - void (*register_synthdef)(Methcla_Host* host, - const Methcla_SynthDef* synthDef); + void (*register_synthdef)(Methcla_Host* host, const Methcla_SynthDef* def); + + //* Register a resource type definition. + void (*register_resource_def)(Methcla_Host* host, + const Methcla_ResourceDef* def); //* Register sound file API. void (*register_soundfile_api)(Methcla_Host* host, @@ -299,12 +302,16 @@ struct Methcla_Host //* Log a message and a newline character. void (*log_line)(Methcla_Host* host, Methcla_LogLevel level, const char* message); - - //* Register a resource type definition. - void (*register_resource_def)(Methcla_Host* host, - const Methcla_ResourceDef* def); }; +static inline void methcla_host_register_synthdef(Methcla_Host* host, + const Methcla_SynthDef* def) +{ + assert(host && host->register_synthdef); + assert(def); + host->register_synthdef(host, def); +} + static inline void methcla_host_register_resource_def(Methcla_Host* host, const Methcla_ResourceDef* def) @@ -314,15 +321,6 @@ methcla_host_register_resource_def(Methcla_Host* host, host->register_resource_def(host, def); } -static inline void -methcla_host_register_synthdef(Methcla_Host* host, - const Methcla_SynthDef* synthDef) -{ - assert(host && host->register_synthdef); - assert(synthDef); - host->register_synthdef(host, synthDef); -} - static inline void methcla_host_register_soundfile_api(Methcla_Host* host, Methcla_SoundFileAPI* api) diff --git a/src/Methcla/Audio/Engine.cpp b/src/Methcla/Audio/Engine.cpp index f36a8316..6a813f13 100644 --- a/src/Methcla/Audio/Engine.cpp +++ b/src/Methcla/Audio/Engine.cpp @@ -42,11 +42,11 @@ namespace { METHCLA_C_LINKAGE void methcla_api_host_register_synthdef(Methcla_Host* host, - const Methcla_SynthDef* synthDef) + const Methcla_SynthDef* def) { assert(host && host->handle); - assert(synthDef); - static_cast(host->handle)->registerSynthDef(synthDef); + assert(def); + static_cast(host->handle)->registerSynthDef(def); } METHCLA_C_LINKAGE void @@ -300,11 +300,12 @@ Environment::Environment(LogHandler logHandler, PacketHandler packetHandler, , m_blockSize(options.blockSize) // Methcla_Host interface , m_host({this, methcla_api_host_register_synthdef, + methcla_api_host_register_resource_def, methcla_api_host_register_soundfile_api, methcla_api_host_alloc, methcla_api_host_free, methcla_api_host_alloc_aligned, methcla_api_host_free_aligned, methcla_api_host_soundfile_open, methcla_api_host_perform_command, methcla_api_host_notify, - methcla_api_host_log_line, methcla_api_host_register_resource_def}) + methcla_api_host_log_line}) // Methcla_World interface , m_world({this, methcla_api_world_samplerate, methcla_api_world_block_size, diff --git a/src/Methcla/Audio/Engine.hpp b/src/Methcla/Audio/Engine.hpp index 405a2982..fdd2a475 100644 --- a/src/Methcla/Audio/Engine.hpp +++ b/src/Methcla/Audio/Engine.hpp @@ -137,7 +137,7 @@ namespace Methcla { namespace Audio { bool hasPendingCommands() const; //* Register SynthDef. - void registerSynthDef(const Methcla_SynthDef* synthDef); + void registerSynthDef(const Methcla_SynthDef* def); //* Register ResourceDef. void registerResourceDef(const Methcla_ResourceDef* def); diff --git a/src/Methcla/Audio/EngineImpl.cpp b/src/Methcla/Audio/EngineImpl.cpp index 9e898c50..df6c45af 100644 --- a/src/Methcla/Audio/EngineImpl.cpp +++ b/src/Methcla/Audio/EngineImpl.cpp @@ -424,6 +424,220 @@ void EnvironmentImpl::processBundle(Methcla_EngineLogFlags logFlags, } } +namespace { + + class ResourceConstructCommand + { + EnvironmentImpl* m_impl; + int32_t m_resourceId; + const Methcla_ResourceDef* m_def; + void* m_options; + void* m_result; + + static void completeOnRT(Environment* env, void* data) + { + auto* self = static_cast(data); + auto& entry = self->m_impl->m_resources[self->m_resourceId]; + entry.def = self->m_def; + entry.data = self->m_result; + + if (entry.freePending) + { + entry.state = EnvironmentImpl::ResourceEntry::State::Destroying; + self->m_impl->scheduleResourceDestroy(self->m_resourceId); + } + else + { + entry.state = EnvironmentImpl::ResourceEntry::State::Live; + self->m_impl->notifyResourceReady(self->m_resourceId); + } + if (self->m_options) + env->rtMem().free(self->m_options); + env->rtMem().free(self); + } + + public: + ResourceConstructCommand(EnvironmentImpl* impl, int32_t resourceId, + const Methcla_ResourceDef* def, void* options) + : m_impl(impl) + , m_resourceId(resourceId) + , m_def(def) + , m_options(options) + , m_result(nullptr) + {} + + void perform(Environment* env) + { + Methcla_Host host(*env); + m_result = Memory::alloc(m_def->instance_size); + if (m_def->construct) + m_def->construct(&host, m_def, m_options, m_result); + env->sendFromWorker(completeOnRT, this); + } + }; + + // Plays two roles in sequence to avoid an extra RT memory allocation: + // first dispatched as ResourceDestroyCommand (NRT destruction work), then + // re-queued as Notification (sends /resource/destroyed). perform() hides + // Notification::perform() intentionally; completeOnRT re-queues via + // static_cast to select the second role. + class ResourceDestroyCommand : public EnvironmentImpl::Notification + { + EnvironmentImpl* m_impl; + int32_t m_resourceId; + const Methcla_ResourceDef* m_def; + void* m_data; + + static void completeOnRT(Environment*, void* data) + { + auto* self = static_cast(data); + self->m_impl->m_resources[self->m_resourceId] = + EnvironmentImpl::ResourceEntry{}; + self->m_impl->sendToWorker( + static_cast(self)); + } + + void notify(Environment* env) override + { + static const char* address = "/resource/destroyed"; + OSCPP::Client::DynamicPacket packet( + OSCPP::Size::message(address, 1) + OSCPP::Size::int32(1)); + packet.openMessage(address, 1); + packet.int32(m_resourceId); + packet.closeMessage(); + env->notify(packet); + } + + public: + ResourceDestroyCommand(EnvironmentImpl* impl, int32_t resourceId, + const Methcla_ResourceDef* def, void* data) + : m_impl(impl) + , m_resourceId(resourceId) + , m_def(def) + , m_data(data) + {} + + void perform(Environment* env) + { + Methcla_Host host(*env); + if (m_def->destroy) + m_def->destroy(&host, m_data); + Memory::free(m_data); + env->sendFromWorker(completeOnRT, this); + } + }; + + class NodeTreeStatisticsCommand + { + public: + struct Statistics + { + Statistics() + : numGroups(0) + , numSynths(0) + {} + size_t numGroups; + size_t numSynths; + }; + + static Statistics collectStatistics(const Group* group, + Statistics stats = Statistics()) + { + stats.numGroups++; + + const Node* cur = group->first(); + + while (cur != nullptr) + { + const Group* subGroup = dynamic_cast(cur); + if (subGroup == nullptr) + { + stats.numSynths++; + } + else + { + stats = collectStatistics(subGroup, stats); + } + cur = cur->next(); + } + + return stats; + } + + NodeTreeStatisticsCommand(Methcla_RequestId requestId, Statistics stats) + : m_requestId(requestId) + , m_stats(stats) + {} + + void perform(Environment* env) + { + static const char* address = "/node/tree/statistics"; + OSCPP::Client::DynamicPacket packet( + OSCPP::Size::message(address, 2) + OSCPP::Size::int32(2)); + packet.openMessage(address, 2); + packet.int32(static_cast(m_stats.numGroups)); + packet.int32(static_cast(m_stats.numSynths)); + packet.closeMessage(); + env->reply(m_requestId, packet); + env->sendFromWorker(perform_rt_free, this); + } + + private: + Methcla_RequestId m_requestId; + Statistics m_stats; + }; + + class RTMemoryStatisticsCommand + { + public: + RTMemoryStatisticsCommand(Methcla_RequestId requestId, + const RTMemoryManager::Statistics& stats) + : m_requestId(requestId) + , m_stats(stats) + {} + + void perform(Environment* env) + { + static const char* address = "/engine/realtime-memory/statistics"; + OSCPP::Client::DynamicPacket packet( + OSCPP::Size::message(address, 2) + OSCPP::Size::int32(2)); + packet.openMessage(address, 2); + packet.int32(static_cast(m_stats.freeNumBytes)); + packet.int32(static_cast(m_stats.usedNumBytes)); + packet.closeMessage(); + env->reply(m_requestId, packet); + env->sendFromWorker(perform_rt_free, this); + } + + private: + Methcla_RequestId m_requestId; + RTMemoryManager::Statistics m_stats; + }; + + class ResourceReadyNotification : public EnvironmentImpl::Notification + { + int32_t m_resourceId; + + public: + explicit ResourceReadyNotification(int32_t resourceId) + : m_resourceId(resourceId) + {} + + private: + void notify(Environment* env) override + { + static const char* address = "/resource/ready"; + OSCPP::Client::DynamicPacket packet( + OSCPP::Size::message(address, 1) + OSCPP::Size::int32(1)); + packet.openMessage(address, 1); + packet.int32(m_resourceId); + packet.closeMessage(); + env->notify(packet); + } + }; + +} // namespace + void EnvironmentImpl::processMessage(Methcla_EngineLogFlags logFlags, const OSCPP::Server::Message& msg, Methcla_Time scheduleTime, @@ -643,151 +857,30 @@ void EnvironmentImpl::processMessage(Methcla_EngineLogFlags logFlags, } else if (msg == "/node/tree/statistics") { - class CommandNodeTreeStatistics - { - public: - struct Statistics - { - Statistics() - : numGroups(0) - , numSynths(0) - {} - - size_t numGroups = 0; - size_t numSynths = 0; - }; - - static Statistics - collectStatistics(const Group* group, - Statistics stats = Statistics()) - { - stats.numGroups++; - - const Node* cur = group->first(); - - while (cur != nullptr) - { - const Group* subGroup = dynamic_cast(cur); - if (subGroup == nullptr) - { - stats.numSynths++; - } - else - { - stats = collectStatistics(subGroup, stats); - } - cur = cur->next(); - } - - return stats; - } - - CommandNodeTreeStatistics(Methcla_RequestId requestId, - Statistics stats) - : m_requestId(requestId) - , m_stats(stats) - {} - - void perform(Environment* env) - { - static const char* address = "/node/tree/statistics"; - OSCPP::Client::DynamicPacket packet( - OSCPP::Size::message(address, 2) + - OSCPP::Size::int32(2)); - packet.openMessage(address, 2); - packet.int32(static_cast(m_stats.numGroups)); - packet.int32(static_cast(m_stats.numSynths)); - packet.closeMessage(); - env->reply(m_requestId, packet); - env->sendFromWorker(perform_rt_free, this); - } - - private: - Methcla_RequestId m_requestId; - Statistics m_stats; - }; - Methcla_RequestId requestId = args.int32(); - - CommandNodeTreeStatistics::Statistics stats = - CommandNodeTreeStatistics::collectStatistics(rootNode()); - - sendToWorker(requestId, stats); + sendToWorker( + requestId, + NodeTreeStatisticsCommand::collectStatistics(rootNode())); } else if (msg == "/resource/new") { - class ResourceConstructCommand - { - EnvironmentImpl* m_impl; - int32_t m_resourceId; - const Methcla_ResourceDef* m_def; - void* m_options; - void* m_result; - - static void completeOnRT(Environment* env, void* data) - { - auto* self = static_cast(data); - auto& entry = self->m_impl->m_resources[self->m_resourceId]; - entry.def = self->m_def; - entry.data = self->m_result; - - if (entry.freePending) - { - entry.state = ResourceEntry::State::Destroying; - self->m_impl->scheduleResourceDestroy( - self->m_resourceId); - } - else - { - entry.state = ResourceEntry::State::Live; - self->m_impl->notifyResourceReady(self->m_resourceId); - } - env->rtMem().free(self); - } - - public: - ResourceConstructCommand(EnvironmentImpl* impl, - int32_t resourceId, - const Methcla_ResourceDef* def, - void* options) - : m_impl(impl) - , m_resourceId(resourceId) - , m_def(def) - , m_options(options) - , m_result(nullptr) - {} - - void perform(Environment* env) - { - Methcla_Host host(*env); - m_result = Memory::alloc(m_def->instance_size); - if (m_def->construct) - m_def->construct(&host, m_def, m_options, m_result); - if (m_options) - env->sendFromWorker(perform_rt_free, m_options); - env->sendFromWorker(completeOnRT, this); - } - }; - - /* args: requestId:i resourceId:i uri:s [options...] */ - args.int32(); // requestId — unused for now + /* args: resourceId:i uri:s [options...] */ const int32_t resourceId = args.int32(); const char* uri = args.string(); auto it = m_resourceDefs.find(uri); if (it == m_resourceDefs.end()) { - notifyResourceError(resourceId, "Unknown resource type"); + throwErrorWith(kMethcla_UnsupportedResourceTypeError, + [&](std::stringstream& s) { + s << "Unknown resource type: " << uri; + }); } else { const Methcla_ResourceDef* def = it->second; - auto& entry = m_resources[resourceId]; + auto& entry = m_resources[resourceId] = ResourceEntry{}; entry.state = ResourceEntry::State::Constructing; - entry.freePending = false; - entry.def = nullptr; - entry.data = nullptr; - entry.refCount = 0; void* options = nullptr; if (def->options_size > 0) @@ -802,54 +895,38 @@ void EnvironmentImpl::processMessage(Methcla_EngineLogFlags logFlags, const int32_t resourceId = args.int32(); if (resourceId < 0 || static_cast(resourceId) >= m_resources.size()) - return; - auto& entry = m_resources[resourceId]; - if (entry.state == ResourceEntry::State::Constructing) { - entry.freePending = true; + throwErrorWith( + kMethcla_ArgumentError, [&](std::stringstream& s) { + s << "Resource id " << resourceId << " out of range"; + }); } - else if (entry.state == ResourceEntry::State::Live && - entry.refCount == 0) + else { - entry.state = ResourceEntry::State::Destroying; - scheduleResourceDestroy(resourceId); + auto& entry = m_resources[resourceId]; + if (entry.state == ResourceEntry::State::Constructing) + { + entry.freePending = true; + } + else if (entry.state == ResourceEntry::State::Live) + { + if (entry.refCount == 0) + { + entry.state = ResourceEntry::State::Destroying; + scheduleResourceDestroy(resourceId); + } + else + { + entry.freePending = true; + } + } } } else if (msg == "/engine/realtime-memory/statistics") { - class CommandRealtimeMemoryStatistics - { - public: - CommandRealtimeMemoryStatistics( - Methcla_RequestId requestId, - const RTMemoryManager::Statistics& stats) - : m_requestId(requestId) - , m_stats(stats) - {} - - void perform(Environment* env) - { - static const char* address = - "/engine/realtime-memory/statistics"; - OSCPP::Client::DynamicPacket packet( - OSCPP::Size::message(address, 2) + - OSCPP::Size::int32(2)); - packet.openMessage(address, 2); - packet.int32(static_cast(m_stats.freeNumBytes)); - packet.int32(static_cast(m_stats.usedNumBytes)); - packet.closeMessage(); - env->reply(m_requestId, packet); - env->sendFromWorker(perform_rt_free, this); - } - - private: - Methcla_RequestId m_requestId; - RTMemoryManager::Statistics m_stats; - }; - - const Methcla_RequestId requestId = args.int32(); - RTMemoryManager::Statistics stats(rtMem().statistics()); - sendToWorker(requestId, stats); + const Methcla_RequestId requestId = args.int32(); + sendToWorker(requestId, + rtMem().statistics()); } } catch (std::exception& e) @@ -871,45 +948,16 @@ void EnvironmentImpl::registerResourceDef(const Methcla_ResourceDef* def) m_resourceDefs[def->uri] = def; } -namespace { - struct ResourceDestroyCommand - { - EnvironmentImpl* impl; - int32_t resourceId; - const Methcla_ResourceDef* def; - void* data; - - static void completeOnRT(Environment* env, void* d) - { - auto* self = static_cast(d); - auto& entry = self->impl->m_resources[self->resourceId]; - entry.state = EnvironmentImpl::ResourceEntry::State::Free; - entry.def = nullptr; - entry.data = nullptr; - self->impl - ->sendToWorker( - self->resourceId); - env->rtMem().free(self); - } - - void perform(Environment* env) - { - Methcla_Host host(*env); - if (def->destroy) - def->destroy(&host, data); - Memory::free(data); - env->sendFromWorker(completeOnRT, this); - } - }; -} // namespace +void EnvironmentImpl::notifyResourceReady(int32_t resourceId) +{ + sendToWorker(resourceId); +} void EnvironmentImpl::scheduleResourceDestroy(int32_t resourceId) { auto& entry = m_resources[resourceId]; - sendToWorker( - perform_perform, - rtMem().construct( - ResourceDestroyCommand{this, resourceId, entry.def, entry.data})); + sendToWorker(this, resourceId, entry.def, + entry.data); } const std::shared_ptr& diff --git a/src/Methcla/Audio/EngineImpl.hpp b/src/Methcla/Audio/EngineImpl.hpp index 6318c717..041eab37 100644 --- a/src/Methcla/Audio/EngineImpl.hpp +++ b/src/Methcla/Audio/EngineImpl.hpp @@ -12,6 +12,7 @@ #include "Methcla/Memory/Manager.hpp" #include "Methcla/Platform.hpp" #include "Methcla/Plugin/Manager.hpp" +#include "Methcla/Utility/Hash.hpp" #include "Methcla/Utility/Macros.h" #include "Methcla/Utility/MessageQueue.hpp" @@ -24,7 +25,6 @@ #include #include #include -#include #include #include @@ -264,7 +264,9 @@ namespace Methcla { namespace Audio { std::list m_soundFileAPIs; using ResourceDefMap = - std::unordered_map; + std::unordered_map; ResourceDefMap m_resourceDefs; struct ResourceEntry @@ -459,94 +461,16 @@ namespace Methcla { namespace Audio { } } - class ResourceErrorNotification : public Notification - { - int32_t m_resourceId; - const char* m_message; // must be a string literal or static string - - public: - ResourceErrorNotification(int32_t resourceId, const char* message) - : m_resourceId(resourceId) - , m_message(message) - {} - - private: - void notify(Environment* env) override - { - static const char* address = "/resource/error"; - OSCPP::Client::DynamicPacket packet( - OSCPP::Size::message(address, 2) + OSCPP::Size::int32(1) + - OSCPP::Size::string(m_message)); - packet.openMessage(address, 2); - packet.int32(m_resourceId); - packet.string(m_message); - packet.closeMessage(); - env->notify(packet); - } - }; - //* Context: RT - void notifyResourceError(int32_t resourceId, const char* message) - { - sendToWorker(resourceId, message); - } - - class ResourceReadyNotification : public Notification - { - int32_t m_resourceId; - - public: - explicit ResourceReadyNotification(int32_t resourceId) - : m_resourceId(resourceId) - {} + void notifyResourceReady(int32_t resourceId); - private: - void notify(Environment* env) override - { - static const char* address = "/resource/ready"; - OSCPP::Client::DynamicPacket packet( - OSCPP::Size::message(address, 1) + OSCPP::Size::int32(1)); - packet.openMessage(address, 1); - packet.int32(m_resourceId); - packet.closeMessage(); - env->notify(packet); - } - }; - - //* Context: RT - void notifyResourceReady(int32_t resourceId) - { - sendToWorker(resourceId); - } - - void registerResourceDef(const Methcla_ResourceDef* def); + //* Context: NRT + void registerResourceDef(const Methcla_ResourceDef* resourceDef); //* Context: RT — schedule NRT destroy + /resource/destroyed // notification. void scheduleResourceDestroy(int32_t resourceId); - class ResourceDestroyedNotification : public Notification - { - int32_t m_resourceId; - - public: - explicit ResourceDestroyedNotification(int32_t resourceId) - : m_resourceId(resourceId) - {} - - private: - void notify(Environment* env) override - { - static const char* address = "/resource/destroyed"; - OSCPP::Client::DynamicPacket packet( - OSCPP::Size::message(address, 1) + OSCPP::Size::int32(1)); - packet.openMessage(address, 1); - packet.int32(m_resourceId); - packet.closeMessage(); - env->notify(packet); - } - }; - //* Context: NRT void reply(Methcla_RequestId requestId, const void* packet, size_t size) { diff --git a/tests/disksampler_tests.cpp b/tests/disksampler_tests.cpp index e01fa65a..dee302bd 100644 --- a/tests/disksampler_tests.cpp +++ b/tests/disksampler_tests.cpp @@ -100,8 +100,8 @@ TEST(Methcla_Engine, issue_113_disksampler) Methcla::SynthId stats; std::tuple bus; - std::get<0>(bus) = engine->audioBusId().alloc(); - std::get<1>(bus) = engine->audioBusId().alloc(); + std::get<0>(bus) = engine->audioBusIdAllocator().alloc(); + std::get<1>(bus) = engine->audioBusIdAllocator().alloc(); { Methcla::Request request(*engine); diff --git a/tests/resource_tests.cpp b/tests/resource_tests.cpp index ba39dfc4..f518ca73 100644 --- a/tests/resource_tests.cpp +++ b/tests/resource_tests.cpp @@ -12,8 +12,8 @@ using namespace Methcla::Tests; -// Helper: allocate resource, wait for /resource/ready, return latch-signalled -// id. +// Wait for /resource/ready on the given id, asserting it arrives within +// timeout. static void awaitReady(Methcla::Engine& engine, Methcla::ResourceId id, const char* uri) { @@ -34,20 +34,16 @@ static void awaitReady(Methcla::Engine& engine, Methcla::ResourceId id, ASSERT_TRUE(latch.wait()) << "Timed out waiting for /resource/ready"; } -// --------------------------------------------------------------------------- -// Slice 1: Unknown URI → /resource/error -// --------------------------------------------------------------------------- - -TEST(ResourceTests, UnknownUriReturnsError) +TEST(ResourceTests, UnknownUriDoesNotNotifyReady) { auto engine = std::make_unique(); engine->start(); AsyncLatch latch; - Methcla::ResourceId id = engine->allocResourceId(); + Methcla::ResourceId id = engine->resourceIdAllocator().alloc(); engine->addNotificationHandler([&, id](const OSCPP::Server::Message& msg) { - if (msg == "/resource/error" && msg.args().int32() == id.id()) + if (msg == "/resource/ready" && msg.args().int32() == id.id()) { latch.signal(); return true; @@ -61,15 +57,11 @@ TEST(ResourceTests, UnknownUriReturnsError) req.closeBundle(); req.send(); - EXPECT_TRUE(latch.wait()); - engine->freeResourceId(id); + EXPECT_FALSE(latch.wait()); + engine->resourceIdAllocator().free(id); engine->stop(); } -// --------------------------------------------------------------------------- -// Slice 2: Known URI → /resource/ready -// --------------------------------------------------------------------------- - TEST(ResourceTests, KnownUriConstructsAndNotifiesReady) { Methcla::EngineOptions opts; @@ -77,17 +69,13 @@ TEST(ResourceTests, KnownUriConstructsAndNotifiesReady) auto engine = std::make_unique(opts); engine->start(); - Methcla::ResourceId id = engine->allocResourceId(); + Methcla::ResourceId id = engine->resourceIdAllocator().alloc(); awaitReady(*engine, id, METHCLA_TEST_RESOURCE_URI); - engine->freeResourceId(id); + engine->resourceIdAllocator().free(id); engine->stop(); } -// --------------------------------------------------------------------------- -// Slice 3: Free live resource → /resource/destroyed -// --------------------------------------------------------------------------- - TEST(ResourceTests, FreeLiveResourceDestroysIt) { Methcla::EngineOptions opts; @@ -95,7 +83,7 @@ TEST(ResourceTests, FreeLiveResourceDestroysIt) auto engine = std::make_unique(opts); engine->start(); - Methcla::ResourceId id = engine->allocResourceId(); + Methcla::ResourceId id = engine->resourceIdAllocator().alloc(); awaitReady(*engine, id, METHCLA_TEST_RESOURCE_URI); AsyncLatch destroyed; @@ -116,14 +104,12 @@ TEST(ResourceTests, FreeLiveResourceDestroysIt) req.send(); } EXPECT_TRUE(destroyed.wait()); - engine->freeResourceId(id); + engine->resourceIdAllocator().free(id); engine->stop(); } -// --------------------------------------------------------------------------- -// Slice 4: Free during construction → /resource/destroyed (no /resource/ready) -// --------------------------------------------------------------------------- - +// Sending /resource/free before construction completes must result in +// /resource/destroyed only (no /resource/ready). TEST(ResourceTests, FreeDuringConstructSetsFreePending) { Methcla::EngineOptions opts; @@ -131,7 +117,7 @@ TEST(ResourceTests, FreeDuringConstructSetsFreePending) auto engine = std::make_unique(opts); engine->start(); - Methcla::ResourceId id = engine->allocResourceId(); + Methcla::ResourceId id = engine->resourceIdAllocator().alloc(); AsyncLatch ready; AsyncLatch destroyed; @@ -153,17 +139,12 @@ TEST(ResourceTests, FreeDuringConstructSetsFreePending) return false; }); - // Send /resource/new and /resource/free back-to-back without waiting. + // Both commands in one bundle so they are processed in the same RT + // callback. { Methcla::Request req(*engine); req.openBundle(); req.resourceNew(id, METHCLA_TEST_RESOURCE_URI); - req.closeBundle(); - req.send(); - } - { - Methcla::Request req(*engine); - req.openBundle(); req.resourceFree(id); req.closeBundle(); req.send(); @@ -172,6 +153,6 @@ TEST(ResourceTests, FreeDuringConstructSetsFreePending) EXPECT_TRUE(destroyed.wait(std::chrono::milliseconds(1000))); // /resource/ready must NOT have been emitted. EXPECT_FALSE(ready.wait(std::chrono::milliseconds(0))); - engine->freeResourceId(id); + engine->resourceIdAllocator().free(id); engine->stop(); } From 82997ee71faa71b15981889b760e37e1c923d898 Mon Sep 17 00:00:00 2001 From: Stefan Kersten Date: Sat, 13 Jun 2026 17:04:11 +0200 Subject: [PATCH 3/5] Re-introduce /resource/error for construct failures; refine resource API - Methcla_ResourceDef::configure now returns Methcla_ErrorCode and construct now returns Methcla_Error; configure failures (RT) are reported via the existing error-log path, construct failures (NRT) emit /resource/error to the client - Wire up configure in the /resource/new handler; only schedule ResourceConstructCommand if configure succeeds - Add ResourceErrorNotification and update ResourceConstructCommand to capture and forward construct errors - Replace Request::resourceNew/resourceFree with resource(uri, options) and free(ResourceId), mirroring the synth/group API; both methods manage the client-side ResourceId allocator automatically - Add resource commands and notifications to docs/osc-api.md --- CHANGELOG.md | 2 +- docs/osc-api.md | 24 +++++++ include/methcla/engine.hpp | 46 +++++++++++-- include/methcla/plugin.h | 20 ++++-- src/Methcla/Audio/EngineImpl.cpp | 91 +++++++++++++++++++++---- tests/resource_tests.cpp | 110 +++++++++++++++++++++---------- tests/test_resource.hpp | 33 ++++++++-- 7 files changed, 262 insertions(+), 64 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d7c8c839..f314c306 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,7 +19,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - `METHCLA_BUILD_TESTS` CMake option, defaults to on when building as top-level project (#122) - CMake presets (`debug`, `release`) and GitHub Actions CI replacing ad-hoc configuration and Travis CI (#118) - Project documentation: CONTRIBUTING guide, architecture overview with Mermaid diagram, OSC API reference, examples README -- Generic RT/NRT shared resource system (#147): `Methcla_ResourceDef` plugin API, `ResourceId` and `allocResourceId`/`freeResourceId` client API, OSC commands `/resource/new` and `/resource/free`, notifications `/resource/ready`, `/resource/error`, `/resource/destroyed`, with `freePending` support for races between construction and free +- Generic RT/NRT shared resource system (#147): `Methcla_ResourceDef` plugin API, client-side `ResourceId` API, OSC commands `/resource/new` and `/resource/free`, notifications `/resource/ready`, `/resource/error`, and `/resource/destroyed` ### Changed diff --git a/docs/osc-api.md b/docs/osc-api.md index da90081f..e1f53ee7 100644 --- a/docs/osc-api.md +++ b/docs/osc-api.md @@ -131,6 +131,30 @@ The engine sends these messages to the host without a corresponding request. The Sent unconditionally when any node is freed (by any means, including **`/node/free`**, done-action flags, or parent group teardown). +## Resource commands + +* **`/resource/new`** `i:resource-id s:uri [options...]` + + Allocate a resource of type `uri` with the given client-assigned `resource-id`. Any trailing arguments are passed to the resource definition's `configure` function. On success the engine sends `/resource/ready` asynchronously once construction on the NRT thread completes. On `configure` error the engine replies with `/error`; on `construct` error it sends `/resource/error`. + +* **`/resource/free`** `i:resource-id` + + Release a live resource. If the resource is still being constructed, the free is deferred until construction completes (`freePending`). The engine sends `/resource/destroyed` once destruction is complete. + +## Resource notifications + +* **`/resource/ready`** `i:resource-id` + + Sent when a resource has been successfully constructed and is ready for use. + +* **`/resource/error`** `i:resource-id i:error-code s:message` + + Sent when a resource's `construct` function returns an error. The resource entry is freed and the id is reusable. `error-code` is a `Methcla_ErrorCode` value. + +* **`/resource/destroyed`** `i:resource-id` + + Sent when a resource has been destroyed in response to `/resource/free`. + ## Error responses * **`/error`** `i:error-code s:message` diff --git a/include/methcla/engine.hpp b/include/methcla/engine.hpp index 23ce49fb..0231423c 100644 --- a/include/methcla/engine.hpp +++ b/include/methcla/engine.hpp @@ -627,6 +627,10 @@ namespace Methcla { BusMappingFlags flags = kBusMappingInternal); inline void set(NodeId node, size_t index, double value); inline void free(NodeId node); + inline ResourceId + resource(const char* uri, + const std::list& options = std::list()); + inline void free(ResourceId id); }; class Request @@ -866,23 +870,37 @@ namespace Methcla { .closeMessage(); } - void resourceNew(ResourceId id, const char* uri) + ResourceId + resource(const char* uri, + const std::list& options = std::list()) { beginMessage(); + + const ResourceId resourceId( + m_engine->resourceIdAllocator().alloc()); + oscPacket() - .openMessage("/resource/new", 2) - .int32(id.id()) - .string(uri) - .closeMessage(); + .openMessage("/resource/new", 2 + options.size()) + .int32(resourceId.id()) + .string(uri); + + for (const auto& x : options) + x.put(oscPacket()); + + oscPacket().closeMessage(); + + return resourceId; } - void resourceFree(ResourceId id) + void free(ResourceId id) { beginMessage(); + oscPacket() .openMessage("/resource/free", 1) .int32(id.id()) .closeMessage(); + m_engine->resourceIdAllocator().free(id); } }; @@ -957,6 +975,22 @@ namespace Methcla { request.send(); } + ResourceId EngineInterface::resource(const char* uri, + const std::list& options) + { + Request request(this); + ResourceId result = request.resource(uri, options); + request.send(); + return result; + } + + void EngineInterface::free(ResourceId id) + { + Request request(this); + request.free(id); + request.send(); + } + class Engine : public EngineInterface { public: diff --git a/include/methcla/plugin.h b/include/methcla/plugin.h index 8adbaddc..16f622c2 100644 --- a/include/methcla/plugin.h +++ b/include/methcla/plugin.h @@ -171,13 +171,19 @@ struct Methcla_ResourceDef //* Mutability hint. Methcla_ResourceMutability mutability; - //* Parse OSC options and fill options struct. - void (*configure)(const void* tag_buffer, size_t tag_size, - const void* arg_buffer, size_t arg_size, void* options); - - //* Construct a resource instance at the given location. - void (*construct)(Methcla_Host* host, const Methcla_ResourceDef* def, - const void* options, void* instance); + //* Parse OSC options and fill options struct. Returns kMethcla_NoError on + //* success. Runs on the RT thread; must not allocate. + Methcla_ErrorCode (*configure)(const void* tag_buffer, size_t tag_size, + const void* arg_buffer, size_t arg_size, + void* options); + + //* Construct a resource instance at the given location. Returns + //* methcla_no_error() on success. On failure the engine takes ownership of + //* the returned Methcla_Error and emits /resource/error; destroy will not + //* be called. + Methcla_Error (*construct)(Methcla_Host* host, + const Methcla_ResourceDef* def, + const void* options, void* instance); //* Destroy a resource instance. void (*destroy)(Methcla_Host* host, void* instance); diff --git a/src/Methcla/Audio/EngineImpl.cpp b/src/Methcla/Audio/EngineImpl.cpp index df6c45af..056e4019 100644 --- a/src/Methcla/Audio/EngineImpl.cpp +++ b/src/Methcla/Audio/EngineImpl.cpp @@ -426,6 +426,38 @@ void EnvironmentImpl::processBundle(Methcla_EngineLogFlags logFlags, namespace { + class ResourceErrorNotification : public EnvironmentImpl::Notification + { + int32_t m_resourceId; + Methcla_Error m_error; + + public: + ResourceErrorNotification(int32_t resourceId, Methcla_Error error) + : m_resourceId(resourceId) + , m_error(error) + {} + + private: + void notify(Environment* env) override + { + constexpr const char* address = "/resource/error"; + const char* msg = methcla_error_message(m_error); + if (!msg) + msg = + methcla_error_code_description(methcla_error_code(m_error)); + OSCPP::Client::DynamicPacket packet( + OSCPP::Size::message(address, 3) + OSCPP::Size::int32(2) + + OSCPP::Size::string(msg)); + packet.openMessage(address, 3); + packet.int32(m_resourceId); + packet.int32(static_cast(methcla_error_code(m_error))); + packet.string(msg); + packet.closeMessage(); + env->notify(packet); + methcla_error_free(m_error); + } + }; + class ResourceConstructCommand { EnvironmentImpl* m_impl; @@ -433,23 +465,36 @@ namespace { const Methcla_ResourceDef* m_def; void* m_options; void* m_result; + Methcla_Error m_error; static void completeOnRT(Environment* env, void* data) { auto* self = static_cast(data); - auto& entry = self->m_impl->m_resources[self->m_resourceId]; - entry.def = self->m_def; - entry.data = self->m_result; - if (entry.freePending) + if (methcla_is_error(self->m_error)) { - entry.state = EnvironmentImpl::ResourceEntry::State::Destroying; - self->m_impl->scheduleResourceDestroy(self->m_resourceId); + self->m_impl->m_resources[self->m_resourceId] = + EnvironmentImpl::ResourceEntry{}; + self->m_impl->sendToWorker( + self->m_resourceId, self->m_error); } else { - entry.state = EnvironmentImpl::ResourceEntry::State::Live; - self->m_impl->notifyResourceReady(self->m_resourceId); + auto& entry = self->m_impl->m_resources[self->m_resourceId]; + entry.def = self->m_def; + entry.data = self->m_result; + + if (entry.freePending) + { + entry.state = + EnvironmentImpl::ResourceEntry::State::Destroying; + self->m_impl->scheduleResourceDestroy(self->m_resourceId); + } + else + { + entry.state = EnvironmentImpl::ResourceEntry::State::Live; + self->m_impl->notifyResourceReady(self->m_resourceId); + } } if (self->m_options) env->rtMem().free(self->m_options); @@ -464,6 +509,7 @@ namespace { , m_def(def) , m_options(options) , m_result(nullptr) + , m_error(methcla_no_error()) {} void perform(Environment* env) @@ -471,7 +517,12 @@ namespace { Methcla_Host host(*env); m_result = Memory::alloc(m_def->instance_size); if (m_def->construct) - m_def->construct(&host, m_def, m_options, m_result); + m_error = m_def->construct(&host, m_def, m_options, m_result); + if (methcla_is_error(m_error)) + { + Memory::free(m_result); + m_result = nullptr; + } env->sendFromWorker(completeOnRT, this); } }; @@ -879,13 +930,31 @@ void EnvironmentImpl::processMessage(Methcla_EngineLogFlags logFlags, else { const Methcla_ResourceDef* def = it->second; - auto& entry = m_resources[resourceId] = ResourceEntry{}; - entry.state = ResourceEntry::State::Constructing; void* options = nullptr; if (def->options_size > 0) options = rtMem().alloc(def->options_size); + if (def->configure) + { + auto state = args.state(); + Methcla_ErrorCode code = def->configure( + std::get<0>(state).pos(), + std::get<0>(state).consumable(), + std::get<1>(state).pos(), + std::get<1>(state).consumable(), options); + if (code != kMethcla_NoError) + { + if (options) + rtMem().free(options); + throwErrorWith(code, [&](std::stringstream& s) { + s << "configure failed for " << uri; + }); + } + } + + auto& entry = m_resources[resourceId] = ResourceEntry{}; + entry.state = ResourceEntry::State::Constructing; sendToWorker(this, resourceId, def, options); } diff --git a/tests/resource_tests.cpp b/tests/resource_tests.cpp index f518ca73..b2b0bd24 100644 --- a/tests/resource_tests.cpp +++ b/tests/resource_tests.cpp @@ -12,11 +12,15 @@ using namespace Methcla::Tests; -// Wait for /resource/ready on the given id, asserting it arrives within -// timeout. -static void awaitReady(Methcla::Engine& engine, Methcla::ResourceId id, - const char* uri) +// Construct a resource of type `uri`, wait for /resource/ready, and return the +// allocated id. +static Methcla::ResourceId awaitReady(Methcla::Engine& engine, const char* uri) { + Methcla::Request req(engine); + req.openBundle(); + const Methcla::ResourceId id = req.resource(uri); + req.closeBundle(); + AsyncLatch latch; engine.addNotificationHandler([&, id](const OSCPP::Server::Message& msg) { if (msg == "/resource/ready" && msg.args().int32() == id.id()) @@ -26,12 +30,9 @@ static void awaitReady(Methcla::Engine& engine, Methcla::ResourceId id, } return false; }); - Methcla::Request req(engine); - req.openBundle(); - req.resourceNew(id, uri); - req.closeBundle(); req.send(); - ASSERT_TRUE(latch.wait()) << "Timed out waiting for /resource/ready"; + EXPECT_TRUE(latch.wait()) << "Timed out waiting for /resource/ready"; + return id; } TEST(ResourceTests, UnknownUriDoesNotNotifyReady) @@ -39,9 +40,12 @@ TEST(ResourceTests, UnknownUriDoesNotNotifyReady) auto engine = std::make_unique(); engine->start(); - AsyncLatch latch; - Methcla::ResourceId id = engine->resourceIdAllocator().alloc(); + Methcla::Request req(*engine); + req.openBundle(); + const Methcla::ResourceId id = req.resource("methcla://plugins/unknown"); + req.closeBundle(); + AsyncLatch latch; engine->addNotificationHandler([&, id](const OSCPP::Server::Message& msg) { if (msg == "/resource/ready" && msg.args().int32() == id.id()) { @@ -50,11 +54,6 @@ TEST(ResourceTests, UnknownUriDoesNotNotifyReady) } return false; }); - - Methcla::Request req(*engine); - req.openBundle(); - req.resourceNew(id, "methcla://plugins/unknown"); - req.closeBundle(); req.send(); EXPECT_FALSE(latch.wait()); @@ -69,9 +68,8 @@ TEST(ResourceTests, KnownUriConstructsAndNotifiesReady) auto engine = std::make_unique(opts); engine->start(); - Methcla::ResourceId id = engine->resourceIdAllocator().alloc(); - awaitReady(*engine, id, METHCLA_TEST_RESOURCE_URI); - + const Methcla::ResourceId id = + awaitReady(*engine, METHCLA_TEST_RESOURCE_URI); engine->resourceIdAllocator().free(id); engine->stop(); } @@ -83,8 +81,8 @@ TEST(ResourceTests, FreeLiveResourceDestroysIt) auto engine = std::make_unique(opts); engine->start(); - Methcla::ResourceId id = engine->resourceIdAllocator().alloc(); - awaitReady(*engine, id, METHCLA_TEST_RESOURCE_URI); + const Methcla::ResourceId id = + awaitReady(*engine, METHCLA_TEST_RESOURCE_URI); AsyncLatch destroyed; engine->addNotificationHandler([&, id](const OSCPP::Server::Message& msg) { @@ -99,11 +97,58 @@ TEST(ResourceTests, FreeLiveResourceDestroysIt) { Methcla::Request req(*engine); req.openBundle(); - req.resourceFree(id); + req.free(id); req.closeBundle(); req.send(); } EXPECT_TRUE(destroyed.wait()); + engine->stop(); +} + +TEST(ResourceTests, ConstructErrorNotifiesResourceError) +{ + Methcla::EngineOptions opts; + opts.addLibrary(methcla_plugins_test_resource); + auto engine = std::make_unique(opts); + engine->start(); + + Methcla::Request req(*engine); + req.openBundle(); + const Methcla::ResourceId id = + req.resource(METHCLA_TEST_RESOURCE_URI, {Methcla::Value(1)}); + req.closeBundle(); + + AsyncLatch error; + AsyncLatch ready; + + engine->addNotificationHandler([&, id](const OSCPP::Server::Message& msg) { + if (msg == "/resource/error") + { + auto args = msg.args(); + if (args.int32() == id.id()) + { + int32_t code = args.int32(); + const char* message = args.string(); + EXPECT_EQ(code, static_cast(kMethcla_ArgumentError)); + EXPECT_STREQ(message, "boom"); + error.signal(); + return true; + } + } + return false; + }); + engine->addNotificationHandler([&, id](const OSCPP::Server::Message& msg) { + if (msg == "/resource/ready" && msg.args().int32() == id.id()) + { + ready.signal(); + return true; + } + return false; + }); + req.send(); + + EXPECT_TRUE(error.wait(std::chrono::milliseconds(1000))); + EXPECT_FALSE(ready.wait(std::chrono::milliseconds(0))); engine->resourceIdAllocator().free(id); engine->stop(); } @@ -117,7 +162,13 @@ TEST(ResourceTests, FreeDuringConstructSetsFreePending) auto engine = std::make_unique(opts); engine->start(); - Methcla::ResourceId id = engine->resourceIdAllocator().alloc(); + // Both commands in one bundle so they are processed in the same RT + // callback. + Methcla::Request req(*engine); + req.openBundle(); + const Methcla::ResourceId id = req.resource(METHCLA_TEST_RESOURCE_URI); + req.free(id); + req.closeBundle(); AsyncLatch ready; AsyncLatch destroyed; @@ -138,21 +189,10 @@ TEST(ResourceTests, FreeDuringConstructSetsFreePending) } return false; }); - - // Both commands in one bundle so they are processed in the same RT - // callback. - { - Methcla::Request req(*engine); - req.openBundle(); - req.resourceNew(id, METHCLA_TEST_RESOURCE_URI); - req.resourceFree(id); - req.closeBundle(); - req.send(); - } + req.send(); EXPECT_TRUE(destroyed.wait(std::chrono::milliseconds(1000))); // /resource/ready must NOT have been emitted. EXPECT_FALSE(ready.wait(std::chrono::milliseconds(0))); - engine->resourceIdAllocator().free(id); engine->stop(); } diff --git a/tests/test_resource.hpp b/tests/test_resource.hpp index 9e98c304..61de072d 100644 --- a/tests/test_resource.hpp +++ b/tests/test_resource.hpp @@ -8,6 +8,8 @@ #include +#include + #define METHCLA_TEST_RESOURCE_URI METHCLA_PLUGINS_URI "/test-resource" namespace TestResourcePlugin { @@ -17,10 +19,33 @@ namespace TestResourcePlugin { int dummy = 0; }; - static void construct(Methcla_Host*, const Methcla_ResourceDef*, - const void*, void* instance) + struct TestResourceOptions + { + bool fail_construct = false; + }; + + static Methcla_ErrorCode configure(const void* tag_buffer, size_t tag_size, + const void* arg_buffer, size_t arg_size, + void* options_ptr) + { + auto* opts = static_cast(options_ptr); + new (opts) TestResourceOptions{}; + OSCPP::Server::ArgStream args(OSCPP::ReadStream(tag_buffer, tag_size), + OSCPP::ReadStream(arg_buffer, arg_size)); + if (!args.atEnd()) + opts->fail_construct = args.int32() != 0; + return kMethcla_NoError; + } + + static Methcla_Error construct(Methcla_Host*, const Methcla_ResourceDef*, + const void* options_ptr, void* instance) { + const auto* opts = static_cast(options_ptr); + if (opts && opts->fail_construct) + return methcla_error_new_with_message(kMethcla_ArgumentError, + "boom"); new (instance) TestResource(); + return methcla_no_error(); } static void destroy(Methcla_Host*, void* instance) @@ -30,9 +55,9 @@ namespace TestResourcePlugin { static const Methcla_ResourceDef def = {METHCLA_TEST_RESOURCE_URI, sizeof(TestResource), - 0, + sizeof(TestResourceOptions), kMethcla_Immutable, - nullptr, + configure, construct, destroy}; From eac0c605e53a70d10ec52d4bff5470a03a96beff Mon Sep 17 00:00:00 2001 From: Stefan Kersten Date: Sat, 13 Jun 2026 18:17:52 +0200 Subject: [PATCH 4/5] Add RT acquire/release primitives, perform_with_resources, and C++ wrappers Completes the remaining #147 surface beyond the resource pool + OSC verbs that landed earlier in #152: Methcla_World::resource_acquire/release (URI-checked, refcount++/--), perform_with_resources (bracketed NRT callback), Methcla::Plugin::ResourceRef RAII wrapper, and Methcla::Plugin::ResourceDef registration template. Adds a Resource system section to docs/architecture.md with a state-machine diagram and threading table; updates CONTEXT.md, CHANGELOG.md, and osc-api.md. AudioBuffer (first concrete Resource class) is now tracked separately in #153. --- CHANGELOG.md | 2 +- CONTEXT.md | 10 +- docs/architecture.md | 61 +++++++++ docs/osc-api.md | 2 +- include/methcla/plugin.h | 70 +++++++++- include/methcla/plugin.hpp | 139 ++++++++++++++++++++ src/Methcla/Audio/Engine.cpp | 48 ++++++- src/Methcla/Audio/Engine.hpp | 12 ++ src/Methcla/Audio/EngineImpl.cpp | 124 ++++++++++++++++++ src/Methcla/Audio/EngineImpl.hpp | 18 +++ tests/resource_tests.cpp | 124 ++++++++++++++++++ tests/test_resource_synth.hpp | 214 +++++++++++++++++++++++++++++++ 12 files changed, 819 insertions(+), 5 deletions(-) create mode 100644 tests/test_resource_synth.hpp diff --git a/CHANGELOG.md b/CHANGELOG.md index f314c306..da6dac3e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,7 +19,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - `METHCLA_BUILD_TESTS` CMake option, defaults to on when building as top-level project (#122) - CMake presets (`debug`, `release`) and GitHub Actions CI replacing ad-hoc configuration and Travis CI (#118) - Project documentation: CONTRIBUTING guide, architecture overview with Mermaid diagram, OSC API reference, examples README -- Generic RT/NRT shared resource system (#147): `Methcla_ResourceDef` plugin API, client-side `ResourceId` API, OSC commands `/resource/new` and `/resource/free`, notifications `/resource/ready`, `/resource/error`, and `/resource/destroyed` +- Generic RT/NRT shared resource system (#147): `Methcla_ResourceDef` plugin API, client-side `ResourceId` API, OSC commands `/resource/new` and `/resource/free`, notifications `/resource/ready`, `/resource/error`, and `/resource/destroyed`, RT-side `methcla_world_resource_acquire`/`methcla_world_resource_release` primitives, NRT bracketed access via `methcla_world_perform_with_resources`, and `ResourceRef` / `ResourceDef` C++ wrappers ### Changed diff --git a/CONTEXT.md b/CONTEXT.md index f60ae4cb..c55d783a 100644 --- a/CONTEXT.md +++ b/CONTEXT.md @@ -24,7 +24,15 @@ _Avoid_: voice, instance, node (overloaded with the Node base class) **AudioBus**: A multi-channel buffer used for audio routing between Synths inside the Engine. -_Avoid_: channel, buffer, bus (acceptable shorthand in code) +_Avoid_: channel, bus (acceptable shorthand in code) + +**Resource**: +A Plugin-registered shared data object managed by the Engine, identified by a `Methcla_ResourceId` integer and typed by URI. Allocated/freed via OSC, acquired/released by Synths on the RT thread, accessed from the NRT context via `perform_with_resources`. +_Avoid_: instance (a Synth is an instance; a Resource is the thing the Plugin allocates), buffer (overloaded) + +**ResourceDef**: +A Resource type registered by a Plugin. Describes the configure, construct, destroy callbacks and the interface URI for one kind of shared Resource. +_Avoid_: resource class, resource type **Soundfile API**: A Plugin category that provides file I/O capabilities to other Plugins (disksampler, sampler). Multiple soundfile API Plugins may be registered; the Engine selects by capability. Platform implementations: ExtAudioFile (macOS), libsndfile (Linux). diff --git a/docs/architecture.md b/docs/architecture.md index 94aa5cc9..ee8689d2 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -33,4 +33,65 @@ graph TB - **Internal AudioBuses** route audio between Synths within the Engine. - The **Root Group** is the top of the node tree. Groups nest arbitrarily; Synths are leaves. - Each Synth maps its audio inputs and outputs to buses via `/synth/map/input` and `/synth/map/output` (see [`osc-api.md`](osc-api.md)). + +## Resource system + +The Engine owns a fixed-size pool of typed, reference-counted Resource slots, indexed by a client-assigned `Methcla_ResourceId`. Capacity is set at startup via `EngineOptions::maxNumResources` (default 256). A Resource is a Plugin-allocated object — its layout is declared in a Plugin-supplied header and identified by a URI — that can be shared between the RT and NRT contexts and across Synths. + +### Resource state machine + +Each slot is in one of four states; all transitions run on the RT thread. + +```mermaid +stateDiagram-v2 + [*] --> Free + Free --> Constructing: /resource/new + Constructing --> Live: NRT construct ok + Constructing --> Free: NRT construct error\n(→ /resource/error) + Constructing --> Destroying: freePending after\nNRT construct ok + Live --> Destroying: /resource/free &&\nrefCount == 0 + Live --> Live: /resource/free &&\nrefCount > 0\n(freePending = true) + Live --> Destroying: refCount drops to 0\n&& freePending + Destroying --> Free: NRT destroy returns\n(→ /resource/destroyed) ``` + +`freePending` records that `/resource/free` arrived while the slot was Constructing or Live-with-refCount>0. The state stays Live until the last `resource_release` drains the refcount; that release then transitions the slot to Destroying and schedules the NRT destroy. + +### Acquire / release and refcount + +`resource_acquire(world, id, expected_uri)` on `Methcla_World`: + +1. Bounds-check `id`; require the slot to be Live. +2. Compare the slot's URI to `expected_uri` (string equality); reject on mismatch. +3. Increment `refCount` and return the resource's data pointer. + +`resource_release(world, id)` decrements `refCount`. If the count reaches zero and `freePending` is set, the slot transitions Live → Destroying and an NRT destroy is scheduled. + +Both primitives mutate state from the RT thread only; no atomics are required. + +### NRT access + +`perform_with_resources(world, ids, num_ids, perform, user_data)` brackets a worker-thread callback with RT-side acquire/release of the listed Resources. The engine acquires each id on RT, dispatches the callback to a worker thread with the array of data pointers, and releases the Resources back on RT after the callback returns. The pointers are valid only for the duration of the callback. If any acquire fails the already-acquired Resources are released and the callback is not invoked. + +### Mutability + +`Methcla_ResourceMutability` (`kMethcla_Mutable` / `kMethcla_Immutable`) is informational metadata declared by the Resource type. The Engine does not enforce read/write access; the Resource class's contract documents it for consumers and Plugin authors. + +### Threading summary + +| Event | Thread | Touches | +| --------------------------------- | -------- | ----------------------------------------------------------------------------- | +| `/resource/new` arrives | RT | slot Free → Constructing; schedule NRT construct | +| NRT construct returns | NRT → RT | RT: slot → Live; emit `/resource/ready` (or Free + `/resource/error`) | +| `resource_acquire` | RT | `refCount++` | +| `resource_release` | RT | `refCount--`; if 0 && freePending: → Destroying + schedule destroy | +| `/resource/free` arrives | RT | refCount==0 && Live: → Destroying + schedule destroy; else `freePending=true` | +| `perform_with_resources` dispatch | RT | acquire all; schedule NRT callback | +| NRT callback returns | NRT → RT | RT auto-releases all | +| NRT destroy returns | NRT → RT | RT: slot → Free; emit `/resource/destroyed` | + +Lifecycle notifications (`/resource/ready`, `/resource/error`, `/resource/destroyed`) originate from RT after the state transition and are dispatched via NRT — mirrors `/node/done`. Sending notifications directly from NRT would let a client's follow-up `/synth/new` race the publish. + +### Multi-worker caveat + +Resource-def callbacks (`construct`, `destroy`, and the `perform_with_resources` callback) run on the worker pool and **can execute concurrently across different worker threads** for different Resources. The engine's NRT-side APIs are thread-safe, but Plugin authors that share state across Resource instances must synchronize that state themselves. diff --git a/docs/osc-api.md b/docs/osc-api.md index e1f53ee7..4b4a2d96 100644 --- a/docs/osc-api.md +++ b/docs/osc-api.md @@ -139,7 +139,7 @@ The engine sends these messages to the host without a corresponding request. The * **`/resource/free`** `i:resource-id` - Release a live resource. If the resource is still being constructed, the free is deferred until construction completes (`freePending`). The engine sends `/resource/destroyed` once destruction is complete. + Release a live resource. If the resource is still being constructed, or held by one or more consumers (refcount > 0), the free is deferred (`freePending`); the engine destroys it once the refcount drains and construction has completed. The engine sends `/resource/destroyed` once destruction is complete. See [architecture.md](architecture.md#resource-system) for the full state machine. ## Resource notifications diff --git a/include/methcla/plugin.h b/include/methcla/plugin.h index 16f622c2..3fa22eb0 100644 --- a/include/methcla/plugin.h +++ b/include/methcla/plugin.h @@ -41,6 +41,22 @@ typedef void (*Methcla_HostPerformFunction)(Methcla_Host* host, void* data); //* Callback function type for performing commands in the realtime context. typedef void (*Methcla_WorldPerformFunction)(Methcla_World* world, void* data); +//* Resource handle exposed by the engine via resource_acquire. Resources are +//* identified by integer id (Methcla_ResourceId, declared below) and typed by +//* URI; consumers cast this opaque pointer to the layout published in the +//* resource type's header after a successful URI-checked acquire. +typedef void Methcla_Resource; + +typedef int32_t Methcla_ResourceId; + +//* Callback function type for accessing one or more resources from the +//* non-realtime context. The engine acquires each listed resource on the RT +//* thread, invokes this callback on a worker thread with the data pointer +//* array, and releases the resources after the callback returns. +typedef void (*Methcla_PerformWithResourcesFunction)( + Methcla_Host* host, Methcla_Resource* const* resources, + size_t num_resources, void* user_data); + //* Realtime interface struct Methcla_World { @@ -72,6 +88,34 @@ struct Methcla_World //* Free synth. void (*synth_done)(Methcla_World* world, Methcla_Synth* synth); + + //* Acquire a Live resource by id, checking that its type URI matches + //* `expected_uri` (compared by string equality). Returns the resource's + //* data pointer (cast by the consumer to the type published in the + //* resource's header) and increments its refcount. Returns NULL if the + //* id is out of range, the slot is not Live, or the URI does not match. + Methcla_Resource* (*resource_acquire)(Methcla_World* world, + Methcla_ResourceId id, + const char* expected_uri); + + //* Decrement the refcount of a resource previously acquired by + //* resource_acquire. Each successful acquire must be paired with exactly + //* one release. + void (*resource_release)(Methcla_World* world, Methcla_ResourceId id); + + //* Bracket a non-realtime callback with RT-side acquire/release of the + //* listed resources. The engine acquires every resource (incrementing + //* its refcount) on the RT thread, dispatches the callback to a worker + //* thread with the data pointer array, and releases the resources back + //* on the RT thread after the callback returns. The resource pointers + //* are valid only for the duration of the callback. If any acquire + //* fails the engine releases the resources it has already acquired and + //* the callback is not invoked. + void (*perform_with_resources)(Methcla_World* world, + const Methcla_ResourceId* ids, + size_t num_ids, + Methcla_PerformWithResourcesFunction perform, + void* user_data); }; static inline double methcla_world_samplerate(const Methcla_World* world) @@ -147,7 +191,31 @@ static inline void methcla_world_synth_done(Methcla_World* world, world->synth_done(world, synth); } -typedef int32_t Methcla_ResourceId; +static inline Methcla_Resource* +methcla_world_resource_acquire(Methcla_World* world, Methcla_ResourceId id, + const char* expected_uri) +{ + assert(world && world->resource_acquire); + assert(expected_uri); + return world->resource_acquire(world, id, expected_uri); +} + +static inline void methcla_world_resource_release(Methcla_World* world, + Methcla_ResourceId id) +{ + assert(world && world->resource_release); + world->resource_release(world, id); +} + +static inline void methcla_world_perform_with_resources( + Methcla_World* world, const Methcla_ResourceId* ids, size_t num_ids, + Methcla_PerformWithResourcesFunction perform, void* user_data) +{ + assert(world && world->perform_with_resources); + assert(perform); + assert(num_ids == 0 || ids != NULL); + world->perform_with_resources(world, ids, num_ids, perform, user_data); +} typedef enum { diff --git a/include/methcla/plugin.hpp b/include/methcla/plugin.hpp index 90f3bfa3..833b3697 100644 --- a/include/methcla/plugin.hpp +++ b/include/methcla/plugin.hpp @@ -299,4 +299,143 @@ namespace Methcla { namespace Plugin { SynthDefFlags Flags = kSynthDefDefaultFlags> using StaticSynthDef = SynthDef, Ports, Flags>; + + // RAII handle for an acquired resource. `Resource` is a wrapper type that + // exposes a static `uri()` and a nested `c_type` typedef naming the C ABI + // layout (see e.g. the AudioBuffer wrapper). Acquire happens in the + // constructor; release happens in the destructor. + // + // The constructed handle is empty (`operator bool() == false`) if the + // engine refused the acquire — typically because the id is not Live or + // its URI does not match the wrapper's. Callers must check before use. + template class ResourceRef + { + public: + using c_type = typename Resource::c_type; + + ResourceRef(Methcla_World* world, Methcla_ResourceId id) + : m_world(world) + , m_id(id) + , m_data(static_cast( + methcla_world_resource_acquire(world, id, Resource::uri()))) + {} + + ~ResourceRef() + { + if (m_data) + methcla_world_resource_release(m_world, m_id); + } + + ResourceRef(const ResourceRef&) = delete; + ResourceRef& operator=(const ResourceRef&) = delete; + + ResourceRef(ResourceRef&& other) noexcept + : m_world(other.m_world) + , m_id(other.m_id) + , m_data(other.m_data) + { + other.m_data = nullptr; + } + + ResourceRef& operator=(ResourceRef&& other) noexcept + { + if (this != &other) + { + if (m_data) + methcla_world_resource_release(m_world, m_id); + m_world = other.m_world; + m_id = other.m_id; + m_data = other.m_data; + other.m_data = nullptr; + } + return *this; + } + + Methcla_ResourceId id() const + { + return m_id; + } + c_type* data() const + { + return m_data; + } + explicit operator bool() const + { + return m_data != nullptr; + } + Resource operator*() const + { + return Resource(m_data); + } + + private: + Methcla_World* m_world; + Methcla_ResourceId m_id; + c_type* m_data; + }; + + // Plugin-author helper that bridges the C ABI for a resource type to a + // C++ class. The Resource class must be constructible from + // (HostContext, const typename Options::Type&). On registration the + // resource def is created with static storage duration so it remains + // valid for the engine's lifetime. + template class ResourceDef + { + static Methcla_ErrorCode configure(const void* tag_buffer, + size_t tag_size, + const void* arg_buffer, + size_t arg_size, void* options) + { + try + { + OSCPP::Server::ArgStream args( + OSCPP::ReadStream(tag_buffer, tag_size), + OSCPP::ReadStream(arg_buffer, arg_size)); + new (options) typename Options::Type(args); + return kMethcla_NoError; + } + catch (...) + { + return kMethcla_ArgumentError; + } + } + + static Methcla_Error construct(Methcla_Host* host, + const Methcla_ResourceDef*, + const void* options, void* instance) + { + try + { + new (instance) Resource( + HostContext(host), + *static_cast(options)); + return methcla_no_error(); + } + catch (const std::exception& e) + { + return methcla_error_new_with_message(kMethcla_ArgumentError, + e.what()); + } + catch (...) + { + return methcla_error_new(kMethcla_ArgumentError); + } + } + + static void destroy(Methcla_Host*, void* instance) + { + static_cast(instance)->~Resource(); + } + + public: + void operator()(Methcla_Host* host, const char* uri, + Methcla_ResourceMutability mutability) + { + static const Methcla_ResourceDef kDef = { + uri, sizeof(Resource), sizeof(typename Options::Type), + mutability, configure, construct, + destroy}; + methcla_host_register_resource_def(host, &kDef); + } + }; }} // namespace Methcla::Plugin diff --git a/src/Methcla/Audio/Engine.cpp b/src/Methcla/Audio/Engine.cpp index 6a813f13..e622f24f 100644 --- a/src/Methcla/Audio/Engine.cpp +++ b/src/Methcla/Audio/Engine.cpp @@ -282,6 +282,31 @@ namespace { assert(synth != nullptr); Synth::fromSynth(synth)->setDone(); } + + METHCLA_C_LINKAGE Methcla_Resource* methcla_api_world_resource_acquire( + Methcla_World* world, Methcla_ResourceId id, const char* expectedUri) + { + assert(world && world->handle); + return static_cast(world->handle) + ->acquireResource(id, expectedUri); + } + + METHCLA_C_LINKAGE void + methcla_api_world_resource_release(Methcla_World* world, + Methcla_ResourceId id) + { + assert(world && world->handle); + static_cast(world->handle)->releaseResource(id); + } + + METHCLA_C_LINKAGE void methcla_api_world_perform_with_resources( + Methcla_World* world, const Methcla_ResourceId* ids, size_t numIds, + Methcla_PerformWithResourcesFunction perform, void* userData) + { + assert(world && world->handle); + static_cast(world->handle) + ->performWithResources(ids, numIds, perform, userData); + } } // namespace extern "C" { @@ -312,7 +337,10 @@ Environment::Environment(LogHandler logHandler, PacketHandler packetHandler, methcla_api_world_current_time, methcla_api_world_alloc, methcla_api_world_free, methcla_api_world_alloc_aligned, methcla_api_world_free_aligned, methcla_api_world_perform_command, - methcla_api_world_log_line, methcla_api_world_synth_done}) + methcla_api_world_log_line, methcla_api_world_synth_done, + methcla_api_world_resource_acquire, + methcla_api_world_resource_release, + methcla_api_world_perform_with_resources}) { m_impl = new EnvironmentImpl(this, logHandler, packetHandler, options, @@ -482,6 +510,24 @@ void Environment::registerResourceDef(const Methcla_ResourceDef* def) m_impl->registerResourceDef(def); } +void* Environment::acquireResource(Methcla_ResourceId id, + const char* expectedUri) +{ + return m_impl->acquireResource(id, expectedUri); +} + +void Environment::releaseResource(Methcla_ResourceId id) +{ + m_impl->releaseResource(id); +} + +void Environment::performWithResources( + const Methcla_ResourceId* ids, size_t numIds, + Methcla_PerformWithResourcesFunction perform, void* userData) +{ + m_impl->performWithResources(ids, numIds, perform, userData); +} + const std::shared_ptr& Environment::synthDef(const char* uri) const { return m_impl->synthDef(uri); diff --git a/src/Methcla/Audio/Engine.hpp b/src/Methcla/Audio/Engine.hpp index fdd2a475..87470a38 100644 --- a/src/Methcla/Audio/Engine.hpp +++ b/src/Methcla/Audio/Engine.hpp @@ -142,6 +142,18 @@ namespace Methcla { namespace Audio { //* Register ResourceDef. void registerResourceDef(const Methcla_ResourceDef* def); + //* Context: RT — acquire a live resource by id with URI check. + void* acquireResource(Methcla_ResourceId id, const char* expectedUri); + + //* Context: RT — release a previously acquired resource. + void releaseResource(Methcla_ResourceId id); + + //* Context: RT — bracket NRT callback with acquire/release of listed + //* resources. + void performWithResources(const Methcla_ResourceId* ids, size_t numIds, + Methcla_PerformWithResourcesFunction perform, + void* userData); + //* Lookup SynthDef const std::shared_ptr& synthDef(const char* uri) const; diff --git a/src/Methcla/Audio/EngineImpl.cpp b/src/Methcla/Audio/EngineImpl.cpp index 056e4019..5cadb9a5 100644 --- a/src/Methcla/Audio/EngineImpl.cpp +++ b/src/Methcla/Audio/EngineImpl.cpp @@ -18,6 +18,7 @@ #include #include +#include #include #include @@ -1029,6 +1030,129 @@ void EnvironmentImpl::scheduleResourceDestroy(int32_t resourceId) entry.data); } +void* EnvironmentImpl::acquireResource(int32_t resourceId, + const char* expectedUri) +{ + if (resourceId < 0 || static_cast(resourceId) >= m_resources.size()) + return nullptr; + + auto& entry = m_resources[resourceId]; + if (entry.state != ResourceEntry::State::Live) + return nullptr; + assert(entry.def != nullptr); + if (expectedUri == nullptr || std::strcmp(entry.def->uri, expectedUri) != 0) + return nullptr; + + entry.refCount++; + return entry.data; +} + +void EnvironmentImpl::releaseResource(int32_t resourceId) +{ + assert(resourceId >= 0 && + static_cast(resourceId) < m_resources.size()); + auto& entry = m_resources[resourceId]; + assert(entry.refCount > 0); + entry.refCount--; + if (entry.refCount == 0 && entry.freePending && + entry.state == ResourceEntry::State::Live) + { + entry.state = ResourceEntry::State::Destroying; + scheduleResourceDestroy(resourceId); + } +} + +namespace { + + // perform_with_resources state. Allocated in RT memory at dispatch time; + // ids are copied into the trailing flexible array so the caller's id + // buffer need not outlive the dispatch. NRT callback runs on a worker + // thread; release of all ids happens back on RT before freeing. + struct PerformWithResourcesCommand + { + EnvironmentImpl* impl; + Methcla_PerformWithResourcesFunction perform; + void* userData; + size_t numIds; + // Followed by: + // Methcla_ResourceId ids[numIds]; + // Methcla_Resource* resources[numIds]; + + Methcla_ResourceId* ids() + { + return reinterpret_cast(this + 1); + } + + Methcla_Resource** resources() + { + return reinterpret_cast(ids() + numIds); + } + + static size_t sizeFor(size_t n) + { + return sizeof(PerformWithResourcesCommand) + + n * sizeof(Methcla_ResourceId) + + n * sizeof(Methcla_Resource*); + } + + static void completeOnRT(Environment* env, void* data) + { + auto* self = static_cast(data); + for (size_t i = 0; i < self->numIds; ++i) + self->impl->releaseResource(self->ids()[i]); + env->rtMem().free(self); + } + + static void runOnNRT(Environment* env, void* data) + { + auto* self = static_cast(data); + Methcla_Host host(*env); + self->perform(&host, self->resources(), self->numIds, + self->userData); + env->sendFromWorker(completeOnRT, self); + } + }; + +} // namespace + +void EnvironmentImpl::performWithResources( + const Methcla_ResourceId* ids, size_t numIds, + Methcla_PerformWithResourcesFunction perform, void* userData) +{ + assert(perform != nullptr); + + void* mem = rtMem().alloc(PerformWithResourcesCommand::sizeFor(numIds)); + auto* cmd = static_cast(mem); + cmd->impl = this; + cmd->perform = perform; + cmd->userData = userData; + cmd->numIds = numIds; + + for (size_t i = 0; i < numIds; ++i) + { + if (ids[i] < 0 || static_cast(ids[i]) >= m_resources.size()) + { + for (size_t j = 0; j < i; ++j) + releaseResource(cmd->ids()[j]); + rtMem().free(mem); + return; + } + auto& entry = m_resources[ids[i]]; + if (entry.state != ResourceEntry::State::Live) + { + for (size_t j = 0; j < i; ++j) + releaseResource(cmd->ids()[j]); + rtMem().free(mem); + return; + } + entry.refCount++; + cmd->ids()[i] = ids[i]; + cmd->resources()[i] = entry.data; + } + + sendToWorker(PerformWithResourcesCommand::runOnNRT, cmd); +} + const std::shared_ptr& EnvironmentImpl::synthDef(const char* uri) const { diff --git a/src/Methcla/Audio/EngineImpl.hpp b/src/Methcla/Audio/EngineImpl.hpp index 041eab37..263a3ac1 100644 --- a/src/Methcla/Audio/EngineImpl.hpp +++ b/src/Methcla/Audio/EngineImpl.hpp @@ -471,6 +471,24 @@ namespace Methcla { namespace Audio { // notification. void scheduleResourceDestroy(int32_t resourceId); + //* Context: RT — type-checked acquire. Returns the resource's data + //* pointer and increments its refcount on success, or nullptr on + //* out-of-range id, non-Live state, or URI mismatch. + void* acquireResource(int32_t resourceId, const char* expectedUri); + + //* Context: RT — decrement refcount. If the resource is flagged + //* freePending and the refcount hits zero, transitions Live → + //* Destroying and schedules the NRT destroy. + void releaseResource(int32_t resourceId); + + //* Context: RT — bracket an NRT callback with acquire/release of + //* the listed ids. Resources are released after the callback + //* returns. If any acquire fails the callback is not invoked and + //* the already-acquired resources are released. + void performWithResources(const Methcla_ResourceId* ids, size_t numIds, + Methcla_PerformWithResourcesFunction perform, + void* userData); + //* Context: NRT void reply(Methcla_RequestId requestId, const void* packet, size_t size) { diff --git a/tests/resource_tests.cpp b/tests/resource_tests.cpp index b2b0bd24..386e8e4a 100644 --- a/tests/resource_tests.cpp +++ b/tests/resource_tests.cpp @@ -4,6 +4,7 @@ #include "methcla_tests.hpp" #include "test_resource.hpp" +#include "test_resource_synth.hpp" #include #include @@ -153,6 +154,129 @@ TEST(ResourceTests, ConstructErrorNotifiesResourceError) engine->stop(); } +// Refcount > 0 must defer the destroy until the last release. The synth holds +// a ref for its full lifetime: /resource/free arrives while held → no destroy; +// /node/free releases the ref → destroy fires. +TEST(ResourceTests, HeldResourceIsDestroyedAfterRelease) +{ + Methcla::EngineOptions opts; + opts.addLibrary(methcla_plugins_test_resource_synths); + auto engine = std::make_unique(opts); + engine->start(); + + const Methcla::ResourceId id = + awaitReady(*engine, METHCLA_TEST_RESOURCE_URI); + + const Methcla::SynthId synth = + engine->synth(METHCLA_TEST_HOLD_SYNTH_URI, + Methcla::NodePlacement::head(engine->root()), {}, + {Methcla::Value(id.id())}); + + AsyncLatch destroyed; + engine->addNotificationHandler([&, id](const OSCPP::Server::Message& msg) { + if (msg == "/resource/destroyed" && msg.args().int32() == id.id()) + { + destroyed.signal(); + return true; + } + return false; + }); + + // /resource/free arrives while the synth holds a ref → must NOT destroy. + engine->free(id); + EXPECT_FALSE(destroyed.wait(std::chrono::milliseconds(100))); + + // Freeing the synth releases the ref → destroy fires. + engine->free(synth); + EXPECT_TRUE(destroyed.wait()); + engine->stop(); +} + +// Acquiring with a non-matching URI must return null. The test synth records +// acquire-failure by leaving `data` null, which is observable via the same +// destroy notification: the synth must NOT have prevented destruction. +TEST(ResourceTests, AcquireWithWrongUriReturnsNull) +{ + Methcla::EngineOptions opts; + opts.addLibrary(methcla_plugins_test_resource_synths); + auto engine = std::make_unique(opts); + engine->start(); + + const Methcla::ResourceId id = + awaitReady(*engine, METHCLA_TEST_RESOURCE_URI); + + // Construct a hold-synth pointing at a bogus id (out of range). The + // synth must construct (acquire returns null is not a synth-construct + // error) but not hold a ref. + const Methcla::SynthId synth = engine->synth( + METHCLA_TEST_HOLD_SYNTH_URI, + Methcla::NodePlacement::head(engine->root()), {}, {Methcla::Value(-1)}); + + AsyncLatch destroyed; + engine->addNotificationHandler([&, id](const OSCPP::Server::Message& msg) { + if (msg == "/resource/destroyed" && msg.args().int32() == id.id()) + { + destroyed.signal(); + return true; + } + return false; + }); + + // /resource/free should destroy immediately — the bogus-id synth never + // acquired our resource. + engine->free(id); + EXPECT_TRUE(destroyed.wait()); + + engine->free(synth); + engine->stop(); +} + +// perform_with_resources brackets a worker-thread callback with RT +// acquire/release. The test synth dispatches one on activate and the +// callback emits a notification carrying both a marker and a flag that the +// acquired pointer matched the one observed at synth-construct time. +TEST(ResourceTests, PerformWithResourcesDispatchesCallback) +{ + Methcla::EngineOptions opts; + opts.addLibrary(methcla_plugins_test_resource_synths); + auto engine = std::make_unique(opts); + engine->start(); + + const Methcla::ResourceId id = + awaitReady(*engine, METHCLA_TEST_RESOURCE_URI); + + constexpr int32_t kMarker = 0xBEEF; + + AsyncLatch dispatched; + int32_t receivedMarker = 0; + int32_t ptrOk = -1; + engine->addNotificationHandler([&](const OSCPP::Server::Message& msg) { + if (msg == "/test/perform-with-resources") + { + auto args = msg.args(); + receivedMarker = args.int32(); + ptrOk = args.int32(); + dispatched.signal(); + return true; + } + return false; + }); + + const Methcla::SynthId synth = + engine->synth(METHCLA_TEST_PERFORM_SYNTH_URI, + Methcla::NodePlacement::head(engine->root()), {}, + {Methcla::Value(id.id()), Methcla::Value(kMarker)}); + engine->activate(synth); + + EXPECT_TRUE(dispatched.wait(std::chrono::milliseconds(1000))); + EXPECT_EQ(receivedMarker, kMarker); + EXPECT_EQ(ptrOk, 1); + + engine->free(synth); + engine->free(id); + engine->stop(); +} + // Sending /resource/free before construction completes must result in // /resource/destroyed only (no /resource/ready). TEST(ResourceTests, FreeDuringConstructSetsFreePending) diff --git a/tests/test_resource_synth.hpp b/tests/test_resource_synth.hpp new file mode 100644 index 00000000..d917e4b8 --- /dev/null +++ b/tests/test_resource_synth.hpp @@ -0,0 +1,214 @@ +// Copyright (C) 2026 Methcla contributors +// +// SPDX-License-Identifier: Apache-2.0 + +#pragma once + +// Test synth plugins that exercise the RT-side resource primitives +// (`methcla_world_resource_acquire`, `methcla_world_resource_release`, +// `methcla_world_perform_with_resources`). Used by resource_tests.cpp; not +// shipped to clients. + +#include "test_resource.hpp" + +#include + +#include +#include +#include + +#include +#include + +// Holds a resource ref for its full lifetime. Options: (int resourceId). +// On construct: acquires the resource id with METHCLA_TEST_RESOURCE_URI; if +// acquire returns null the synth records the failure and produces no output. +// On destroy: releases if acquired. +#define METHCLA_TEST_HOLD_SYNTH_URI METHCLA_PLUGINS_URI "/test-hold-resource" + +namespace TestHoldSynth { + + struct Options + { + int32_t resourceId; + }; + + struct Synth + { + Methcla_ResourceId id; + void* data; + }; + + static void configure(const void* tag_buffer, size_t tag_size, + const void* arg_buffer, size_t arg_size, + Methcla_SynthOptions* options) + { + auto* opts = static_cast(options); + new (opts) Options{}; + OSCPP::Server::ArgStream args(OSCPP::ReadStream(tag_buffer, tag_size), + OSCPP::ReadStream(arg_buffer, arg_size)); + opts->resourceId = args.int32(); + } + + static bool port_descriptor(const Methcla_SynthOptions*, Methcla_PortCount, + Methcla_PortDescriptor*) + { + return false; + } + + static void construct(Methcla_World* world, const Methcla_SynthDef*, + const Methcla_SynthOptions* options, + Methcla_Synth* instance) + { + const auto* opts = static_cast(options); + auto* self = new (instance) Synth{}; + self->id = opts->resourceId; + self->data = methcla_world_resource_acquire(world, self->id, + METHCLA_TEST_RESOURCE_URI); + } + + static void connect(Methcla_Synth*, Methcla_PortCount, void*) + {} + + static void activate(Methcla_World*, Methcla_Synth*) + {} + + static void process(Methcla_World*, Methcla_Synth*, size_t) + {} + + static void destroy(Methcla_World* world, Methcla_Synth* instance) + { + auto* self = static_cast(instance); + if (self->data) + methcla_world_resource_release(world, self->id); + self->~Synth(); + } + + static const Methcla_SynthDef def = {METHCLA_TEST_HOLD_SYNTH_URI, + sizeof(Synth), + sizeof(Options), + configure, + port_descriptor, + construct, + connect, + activate, + process, + destroy}; + +} // namespace TestHoldSynth + +// On activate, dispatches perform_with_resources for one resource. The NRT +// callback emits `/test/perform-with-resources ` so +// tests can observe both the dispatch and that the acquired pointer was +// non-null. Options: (int resourceId, int synthMarker). +#define METHCLA_TEST_PERFORM_SYNTH_URI \ + METHCLA_PLUGINS_URI "/test-perform-with-resources" + +namespace TestPerformSynth { + + struct Options + { + int32_t resourceId; + int32_t marker; + }; + + struct Synth + { + Methcla_ResourceId id; + int32_t marker; + int32_t dispatched; + int32_t _pad; + }; + + static void configure(const void* tag_buffer, size_t tag_size, + const void* arg_buffer, size_t arg_size, + Methcla_SynthOptions* options) + { + auto* opts = static_cast(options); + new (opts) Options{}; + OSCPP::Server::ArgStream args(OSCPP::ReadStream(tag_buffer, tag_size), + OSCPP::ReadStream(arg_buffer, arg_size)); + opts->resourceId = args.int32(); + opts->marker = args.int32(); + } + + static bool port_descriptor(const Methcla_SynthOptions*, Methcla_PortCount, + Methcla_PortDescriptor*) + { + return false; + } + + static void construct(Methcla_World*, const Methcla_SynthDef*, + const Methcla_SynthOptions* options, + Methcla_Synth* instance) + { + const auto* opts = static_cast(options); + auto* self = new (instance) Synth{}; + self->id = opts->resourceId; + self->marker = opts->marker; + self->dispatched = 0; + } + + static void connect(Methcla_Synth*, Methcla_PortCount, void*) + {} + + static void onNRT(Methcla_Host* host, Methcla_Resource* const* resources, + size_t num_resources, void* user_data) + { + // Marker passed through the void* parameter (test-only convenience — + // avoids cross-pool allocation lifetimes). + const int32_t marker = + static_cast(reinterpret_cast(user_data)); + const int32_t ptrOk = + (num_resources == 1 && resources[0] != nullptr) ? 1 : 0; + const char* address = "/test/perform-with-resources"; + OSCPP::Client::DynamicPacket packet(OSCPP::Size::message(address, 2) + + OSCPP::Size::int32(2)); + packet.openMessage(address, 2) + .int32(marker) + .int32(ptrOk) + .closeMessage(); + host->notify(host, packet.data(), packet.size()); + } + + static void activate(Methcla_World* world, Methcla_Synth* instance) + { + auto* self = static_cast(instance); + if (self->dispatched) + return; + self->dispatched = true; + void* userData = + reinterpret_cast(static_cast(self->marker)); + methcla_world_perform_with_resources(world, &self->id, 1, onNRT, + userData); + } + + static void process(Methcla_World*, Methcla_Synth*, size_t) + {} + + static void destroy(Methcla_World*, Methcla_Synth* instance) + { + static_cast(instance)->~Synth(); + } + + static const Methcla_SynthDef def = {METHCLA_TEST_PERFORM_SYNTH_URI, + sizeof(Synth), + sizeof(Options), + configure, + port_descriptor, + construct, + connect, + activate, + process, + destroy}; + +} // namespace TestPerformSynth + +static Methcla_Library* methcla_plugins_test_resource_synths(Methcla_Host* host, + const char*) +{ + methcla_host_register_resource_def(host, &TestResourcePlugin::def); + methcla_host_register_synthdef(host, &TestHoldSynth::def); + methcla_host_register_synthdef(host, &TestPerformSynth::def); + return nullptr; +} From 0e90787fae746a9cfdd878ecadfa3c8ad6e73535 Mon Sep 17 00:00:00 2001 From: Stefan Kersten Date: Tue, 16 Jun 2026 23:57:29 +0200 Subject: [PATCH 5/5] Tighten encapsulation of resource-system internals - PerformWithResourcesCommand is now a class with private state and a single public dispatch() entry point; the trailing-flex-array bookkeeping (idsArray/resourcesArray/sizeFor/runOnNRT/completeOnRT) is encapsulated. EnvironmentImpl::performWithResources is a one-liner. - ResourceEntry fields adopt the m_ prefix used by the rest of the module; the struct stays public-by-default because direct access is needed throughout EngineImpl (RT-thread-confined). --- docs/architecture.md | 32 ++++-- include/methcla/plugin.hpp | 66 +++++++++++- src/Methcla/Audio/EngineImpl.cpp | 169 ++++++++++++++++--------------- src/Methcla/Audio/EngineImpl.hpp | 13 ++- 4 files changed, 178 insertions(+), 102 deletions(-) diff --git a/docs/architecture.md b/docs/architecture.md index ee8689d2..88c24004 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -42,17 +42,27 @@ The Engine owns a fixed-size pool of typed, reference-counted Resource slots, in Each slot is in one of four states; all transitions run on the RT thread. -```mermaid -stateDiagram-v2 - [*] --> Free - Free --> Constructing: /resource/new - Constructing --> Live: NRT construct ok - Constructing --> Free: NRT construct error\n(→ /resource/error) - Constructing --> Destroying: freePending after\nNRT construct ok - Live --> Destroying: /resource/free &&\nrefCount == 0 - Live --> Live: /resource/free &&\nrefCount > 0\n(freePending = true) - Live --> Destroying: refCount drops to 0\n&& freePending - Destroying --> Free: NRT destroy returns\n(→ /resource/destroyed) +``` + /resource/new (validated, id reserved) + Free ──────────────────────────────────────────→ Constructing + ▲ │ + │ │ NRT construct returns + │ ┌─────────┴─────────┐ + │ │ │ + │ error ok + │ │ │ + │ NRT destroy returns ▼ ▼ + │ (state → Free) (notify Live + │ /resource/error) │ + │ │ │ /resource/free + │ │ │ refCount==0 → Destroying + │ ▼ │ refCount>0 → freePending=true, + └────────────────────────────────────── Free │ drain via release path + ▼ + Destroying + │ + │ NRT destroy returns + └────────→ Free ``` `freePending` records that `/resource/free` arrived while the slot was Constructing or Live-with-refCount>0. The state stays Live until the last `resource_release` drains the refcount; that release then transitions the slot to Destroying and schedules the NRT destroy. diff --git a/include/methcla/plugin.hpp b/include/methcla/plugin.hpp index 833b3697..3c765d7f 100644 --- a/include/methcla/plugin.hpp +++ b/include/methcla/plugin.hpp @@ -10,6 +10,8 @@ #include #include +#include +#include #include @@ -302,8 +304,8 @@ namespace Methcla { namespace Plugin { // RAII handle for an acquired resource. `Resource` is a wrapper type that // exposes a static `uri()` and a nested `c_type` typedef naming the C ABI - // layout (see e.g. the AudioBuffer wrapper). Acquire happens in the - // constructor; release happens in the destructor. + // layout. Acquire happens in the constructor; release happens in the + // destructor. // // The constructed handle is empty (`operator bool() == false`) if the // engine refused the acquire — typically because the id is not Live or @@ -394,10 +396,34 @@ namespace Methcla { namespace Plugin { new (options) typename Options::Type(args); return kMethcla_NoError; } - catch (...) + catch (const std::invalid_argument&) + { + return kMethcla_ArgumentError; + } + catch (const std::out_of_range&) + { + return kMethcla_ArgumentError; + } + catch (const std::domain_error&) { return kMethcla_ArgumentError; } + catch (const std::logic_error&) + { + return kMethcla_LogicError; + } + catch (const std::bad_alloc&) + { + return kMethcla_MemoryError; + } + catch (const std::system_error&) + { + return kMethcla_SystemError; + } + catch (...) + { + return kMethcla_UnspecifiedError; + } } static Methcla_Error construct(Methcla_Host* host, @@ -411,14 +437,44 @@ namespace Methcla { namespace Plugin { *static_cast(options)); return methcla_no_error(); } - catch (const std::exception& e) + catch (const std::invalid_argument& e) + { + return methcla_error_new_with_message(kMethcla_ArgumentError, + e.what()); + } + catch (const std::out_of_range& e) + { + return methcla_error_new_with_message(kMethcla_ArgumentError, + e.what()); + } + catch (const std::domain_error& e) { return methcla_error_new_with_message(kMethcla_ArgumentError, e.what()); } + catch (const std::logic_error& e) + { + return methcla_error_new_with_message(kMethcla_LogicError, + e.what()); + } + catch (const std::bad_alloc& e) + { + return methcla_error_new_with_message(kMethcla_MemoryError, + e.what()); + } + catch (const std::system_error& e) + { + return methcla_error_new_with_message(kMethcla_SystemError, + e.what()); + } + catch (const std::exception& e) + { + return methcla_error_new_with_message(kMethcla_UnspecifiedError, + e.what()); + } catch (...) { - return methcla_error_new(kMethcla_ArgumentError); + return methcla_error_new(kMethcla_UnspecifiedError); } } diff --git a/src/Methcla/Audio/EngineImpl.cpp b/src/Methcla/Audio/EngineImpl.cpp index 5cadb9a5..dc0f7f3d 100644 --- a/src/Methcla/Audio/EngineImpl.cpp +++ b/src/Methcla/Audio/EngineImpl.cpp @@ -482,18 +482,18 @@ namespace { else { auto& entry = self->m_impl->m_resources[self->m_resourceId]; - entry.def = self->m_def; - entry.data = self->m_result; + entry.m_def = self->m_def; + entry.m_data = self->m_result; - if (entry.freePending) + if (entry.m_freePending) { - entry.state = + entry.m_state = EnvironmentImpl::ResourceEntry::State::Destroying; self->m_impl->scheduleResourceDestroy(self->m_resourceId); } else { - entry.state = EnvironmentImpl::ResourceEntry::State::Live; + entry.m_state = EnvironmentImpl::ResourceEntry::State::Live; self->m_impl->notifyResourceReady(self->m_resourceId); } } @@ -955,7 +955,7 @@ void EnvironmentImpl::processMessage(Methcla_EngineLogFlags logFlags, } auto& entry = m_resources[resourceId] = ResourceEntry{}; - entry.state = ResourceEntry::State::Constructing; + entry.m_state = ResourceEntry::State::Constructing; sendToWorker(this, resourceId, def, options); } @@ -974,20 +974,20 @@ void EnvironmentImpl::processMessage(Methcla_EngineLogFlags logFlags, else { auto& entry = m_resources[resourceId]; - if (entry.state == ResourceEntry::State::Constructing) + if (entry.m_state == ResourceEntry::State::Constructing) { - entry.freePending = true; + entry.m_freePending = true; } - else if (entry.state == ResourceEntry::State::Live) + else if (entry.m_state == ResourceEntry::State::Live) { - if (entry.refCount == 0) + if (entry.m_refCount == 0) { - entry.state = ResourceEntry::State::Destroying; + entry.m_state = ResourceEntry::State::Destroying; scheduleResourceDestroy(resourceId); } else { - entry.freePending = true; + entry.m_freePending = true; } } } @@ -1026,8 +1026,8 @@ void EnvironmentImpl::notifyResourceReady(int32_t resourceId) void EnvironmentImpl::scheduleResourceDestroy(int32_t resourceId) { auto& entry = m_resources[resourceId]; - sendToWorker(this, resourceId, entry.def, - entry.data); + sendToWorker(this, resourceId, entry.m_def, + entry.m_data); } void* EnvironmentImpl::acquireResource(int32_t resourceId, @@ -1037,14 +1037,15 @@ void* EnvironmentImpl::acquireResource(int32_t resourceId, return nullptr; auto& entry = m_resources[resourceId]; - if (entry.state != ResourceEntry::State::Live) + if (entry.m_state != ResourceEntry::State::Live) return nullptr; - assert(entry.def != nullptr); - if (expectedUri == nullptr || std::strcmp(entry.def->uri, expectedUri) != 0) + assert(entry.m_def != nullptr); + if (expectedUri == nullptr || + std::strcmp(entry.m_def->uri, expectedUri) != 0) return nullptr; - entry.refCount++; - return entry.data; + entry.m_refCount++; + return entry.m_data; } void EnvironmentImpl::releaseResource(int32_t resourceId) @@ -1052,42 +1053,63 @@ void EnvironmentImpl::releaseResource(int32_t resourceId) assert(resourceId >= 0 && static_cast(resourceId) < m_resources.size()); auto& entry = m_resources[resourceId]; - assert(entry.refCount > 0); - entry.refCount--; - if (entry.refCount == 0 && entry.freePending && - entry.state == ResourceEntry::State::Live) + assert(entry.m_refCount > 0); + entry.m_refCount--; + if (entry.m_refCount == 0 && entry.m_freePending && + entry.m_state == ResourceEntry::State::Live) { - entry.state = ResourceEntry::State::Destroying; + entry.m_state = ResourceEntry::State::Destroying; scheduleResourceDestroy(resourceId); } } namespace { - // perform_with_resources state. Allocated in RT memory at dispatch time; - // ids are copied into the trailing flexible array so the caller's id - // buffer need not outlive the dispatch. NRT callback runs on a worker - // thread; release of all ids happens back on RT before freeing. - struct PerformWithResourcesCommand + // Command state for perform_with_resources. Allocated in RT memory at + // dispatch time with two trailing flexible arrays — ids and resolved + // resource pointers — so the caller's id buffer need not outlive the + // dispatch. NRT callback runs on a worker thread; release of all ids + // happens back on RT before freeing. + class PerformWithResourcesCommand { - EnvironmentImpl* impl; - Methcla_PerformWithResourcesFunction perform; - void* userData; - size_t numIds; - // Followed by: - // Methcla_ResourceId ids[numIds]; - // Methcla_Resource* resources[numIds]; - - Methcla_ResourceId* ids() + public: + // Allocate, RT-acquire each listed resource, and schedule the NRT + // callback. On any acquire failure releases the already-acquired + // resources, frees the allocation, and returns without dispatching. + static void dispatch(EnvironmentImpl* impl, + const Methcla_ResourceId* ids, size_t numIds, + Methcla_PerformWithResourcesFunction perform, + void* userData) { - return reinterpret_cast(this + 1); - } + void* mem = impl->rtMem().alloc(sizeFor(numIds)); + auto* self = static_cast(mem); + self->m_impl = impl; + self->m_perform = perform; + self->m_userData = userData; + self->m_numIds = numIds; + + for (size_t i = 0; i < numIds; ++i) + { + auto& resources = impl->m_resources; + if (ids[i] < 0 || + static_cast(ids[i]) >= resources.size() || + resources[ids[i]].m_state != + EnvironmentImpl::ResourceEntry::State::Live) + { + for (size_t j = 0; j < i; ++j) + impl->releaseResource(self->idsArray()[j]); + impl->rtMem().free(self); + return; + } + resources[ids[i]].m_refCount++; + self->idsArray()[i] = ids[i]; + self->resourcesArray()[i] = resources[ids[i]].m_data; + } - Methcla_Resource** resources() - { - return reinterpret_cast(ids() + numIds); + impl->sendToWorker(runOnNRT, self); } + private: static size_t sizeFor(size_t n) { return sizeof(PerformWithResourcesCommand) + @@ -1095,22 +1117,37 @@ namespace { n * sizeof(Methcla_Resource*); } - static void completeOnRT(Environment* env, void* data) + Methcla_ResourceId* idsArray() { - auto* self = static_cast(data); - for (size_t i = 0; i < self->numIds; ++i) - self->impl->releaseResource(self->ids()[i]); - env->rtMem().free(self); + return reinterpret_cast(this + 1); + } + + Methcla_Resource** resourcesArray() + { + return reinterpret_cast(idsArray() + m_numIds); } static void runOnNRT(Environment* env, void* data) { auto* self = static_cast(data); Methcla_Host host(*env); - self->perform(&host, self->resources(), self->numIds, - self->userData); + self->m_perform(&host, self->resourcesArray(), self->m_numIds, + self->m_userData); env->sendFromWorker(completeOnRT, self); } + + static void completeOnRT(Environment* env, void* data) + { + auto* self = static_cast(data); + for (size_t i = 0; i < self->m_numIds; ++i) + self->m_impl->releaseResource(self->idsArray()[i]); + env->rtMem().free(self); + } + + EnvironmentImpl* m_impl; + Methcla_PerformWithResourcesFunction m_perform; + void* m_userData; + size_t m_numIds; }; } // namespace @@ -1120,37 +1157,7 @@ void EnvironmentImpl::performWithResources( Methcla_PerformWithResourcesFunction perform, void* userData) { assert(perform != nullptr); - - void* mem = rtMem().alloc(PerformWithResourcesCommand::sizeFor(numIds)); - auto* cmd = static_cast(mem); - cmd->impl = this; - cmd->perform = perform; - cmd->userData = userData; - cmd->numIds = numIds; - - for (size_t i = 0; i < numIds; ++i) - { - if (ids[i] < 0 || static_cast(ids[i]) >= m_resources.size()) - { - for (size_t j = 0; j < i; ++j) - releaseResource(cmd->ids()[j]); - rtMem().free(mem); - return; - } - auto& entry = m_resources[ids[i]]; - if (entry.state != ResourceEntry::State::Live) - { - for (size_t j = 0; j < i; ++j) - releaseResource(cmd->ids()[j]); - rtMem().free(mem); - return; - } - entry.refCount++; - cmd->ids()[i] = ids[i]; - cmd->resources()[i] = entry.data; - } - - sendToWorker(PerformWithResourcesCommand::runOnNRT, cmd); + PerformWithResourcesCommand::dispatch(this, ids, numIds, perform, userData); } const std::shared_ptr& diff --git a/src/Methcla/Audio/EngineImpl.hpp b/src/Methcla/Audio/EngineImpl.hpp index 263a3ac1..8cf7e3e6 100644 --- a/src/Methcla/Audio/EngineImpl.hpp +++ b/src/Methcla/Audio/EngineImpl.hpp @@ -269,6 +269,9 @@ namespace Methcla { namespace Audio { Utility::Hash::cstr_equal>; ResourceDefMap m_resourceDefs; + // Module-internal slot record for the resource pool. All mutation is + // confined to EnvironmentImpl on the RT thread; fields are public for + // direct access by the OSC handlers and command classes below. struct ResourceEntry { enum class State @@ -278,11 +281,11 @@ namespace Methcla { namespace Audio { Live, Destroying }; - State state = State::Free; - bool freePending = false; - const Methcla_ResourceDef* def = nullptr; - void* data = nullptr; - size_t refCount = 0; + State m_state = State::Free; + bool m_freePending = false; + const Methcla_ResourceDef* m_def = nullptr; + void* m_data = nullptr; + size_t m_refCount = 0; }; std::vector m_resources;