From 4de692ccf8c97cb156029375a10fc6a2aa2928fe Mon Sep 17 00:00:00 2001 From: David Robillard Date: Wed, 11 Nov 2020 23:24:34 +0100 Subject: Fix potential state memory leaks --- src/server/BlockImpl.hpp | 3 +- src/server/LV2Block.cpp | 94 ++++++++++++++++++++++----------------- src/server/LV2Block.hpp | 4 +- src/server/State.hpp | 33 ++++++++++++++ src/server/events/CreateBlock.cpp | 4 +- src/server/events/Delta.cpp | 12 ++--- src/server/events/Delta.hpp | 3 +- 7 files changed, 101 insertions(+), 52 deletions(-) create mode 100644 src/server/State.hpp (limited to 'src') diff --git a/src/server/BlockImpl.hpp b/src/server/BlockImpl.hpp index a5f5963a..f6c72c8e 100644 --- a/src/server/BlockImpl.hpp +++ b/src/server/BlockImpl.hpp @@ -20,6 +20,7 @@ #include "BufferRef.hpp" #include "NodeImpl.hpp" #include "PortType.hpp" +#include "State.hpp" #include "types.hpp" #include "ingen/Node.hpp" @@ -108,7 +109,7 @@ public: void set_enabled(bool e) { _enabled = e; } /** Load a preset from the world for this block. */ - virtual LilvState* load_preset(const URI& uri) { return nullptr; } + virtual StatePtr load_preset(const URI& uri) { return {}; } /** Restore `state`. */ virtual void diff --git a/src/server/LV2Block.cpp b/src/server/LV2Block.cpp index 3d180360..787488d5 100644 --- a/src/server/LV2Block.cpp +++ b/src/server/LV2Block.cpp @@ -454,9 +454,10 @@ LV2Block::instantiate(BufferFactory& bufs, const LilvState* state) } // Load initial state if no state is explicitly given - LilvState* default_state = nullptr; + StatePtr default_state{}; if (!state) { - state = default_state = load_preset(_lv2_plugin->uri()); + default_state = load_preset(_lv2_plugin->uri()); + state = default_state.get(); } // Apply state @@ -464,10 +465,6 @@ LV2Block::instantiate(BufferFactory& bufs, const LilvState* state) apply_state(nullptr, state); } - if (default_state) { - lilv_state_free(default_state); - } - // FIXME: Polyphony + worker? if (lilv_plugin_has_feature(plug, uris.work_schedule)) { _worker_iface = static_cast( @@ -484,29 +481,33 @@ LV2Block::save_state(const FilePath& dir) const World& world = _lv2_plugin->world(); LilvWorld* lworld = world.lilv_world(); - LilvState* state = lilv_state_new_from_instance( - _lv2_plugin->lilv_plugin(), const_cast(this)->instance(0), - &world.uri_map().urid_map(), - nullptr, dir.c_str(), dir.c_str(), dir.c_str(), nullptr, nullptr, - LV2_STATE_IS_POD|LV2_STATE_IS_PORTABLE, nullptr); + StatePtr state{ + lilv_state_new_from_instance(_lv2_plugin->lilv_plugin(), + const_cast(this)->instance(0), + &world.uri_map().urid_map(), + nullptr, + dir.c_str(), + dir.c_str(), + dir.c_str(), + nullptr, + nullptr, + LV2_STATE_IS_POD | LV2_STATE_IS_PORTABLE, + nullptr)}; if (!state) { return false; - } else if (lilv_state_get_num_properties(state) == 0) { - lilv_state_free(state); + } else if (lilv_state_get_num_properties(state.get()) == 0) { return false; } lilv_state_save(lworld, &world.uri_map().urid_map(), &world.uri_map().urid_unmap(), - state, + state.get(), nullptr, dir.c_str(), "state.ttl"); - lilv_state_free(state); - return true; } @@ -518,14 +519,22 @@ LV2Block::duplicate(Engine& engine, const SampleRate rate = engine.sample_rate(); // Get current state - LilvState* state = lilv_state_new_from_instance( - _lv2_plugin->lilv_plugin(), instance(0), - &engine.world().uri_map().urid_map(), - nullptr, nullptr, nullptr, nullptr, nullptr, nullptr, LV2_STATE_IS_NATIVE, nullptr); + StatePtr state{ + lilv_state_new_from_instance(_lv2_plugin->lilv_plugin(), + instance(0), + &engine.world().uri_map().urid_map(), + nullptr, + nullptr, + nullptr, + nullptr, + nullptr, + nullptr, + LV2_STATE_IS_NATIVE, + nullptr)}; // Duplicate and instantiate block auto* dup = new LV2Block(_lv2_plugin, symbol, _polyphonic, parent, rate); - if (!dup->instantiate(*engine.buffer_factory(), state)) { + if (!dup->instantiate(*engine.buffer_factory(), state.get())) { delete dup; return nullptr; } @@ -623,7 +632,7 @@ LV2Block::post_process(RunContext& ctx) BlockImpl::post_process(ctx); } -LilvState* +StatePtr LV2Block::load_preset(const URI& uri) { World& world = _lv2_plugin->world(); @@ -634,25 +643,22 @@ LV2Block::load_preset(const URI& uri) lilv_world_load_resource(lworld, preset); // Load preset from world - LV2_URID_Map* map = &world.uri_map().urid_map(); - LilvState* state = lilv_state_new_from_world(lworld, map, preset); + LV2_URID_Map* map = &world.uri_map().urid_map(); + StatePtr state{lilv_state_new_from_world(lworld, map, preset)}; lilv_node_free(preset); return state; } -LilvState* +StatePtr LV2Block::load_state(World& world, const FilePath& path) { LilvWorld* lworld = world.lilv_world(); const URI uri = URI(path); LilvNode* subject = lilv_new_uri(lworld, uri.c_str()); - LilvState* state = lilv_state_new_from_file( - lworld, - &world.uri_map().urid_map(), - subject, - path.c_str()); + StatePtr state{lilv_state_new_from_file( + lworld, &world.uri_map().urid_map(), subject, path.c_str())}; lilv_node_free(subject); return state; @@ -709,25 +715,31 @@ LV2Block::save_preset(const URI& uri, const FilePath dirname = path.parent_path(); const FilePath basename = path.stem(); - LilvState* state = lilv_state_new_from_instance( - _lv2_plugin->lilv_plugin(), instance(0), lmap, - nullptr, nullptr, nullptr, path.c_str(), - get_port_value, this, LV2_STATE_IS_NATIVE, nullptr); + StatePtr state{lilv_state_new_from_instance(_lv2_plugin->lilv_plugin(), + instance(0), + lmap, + nullptr, + nullptr, + nullptr, + path.c_str(), + get_port_value, + this, + LV2_STATE_IS_NATIVE, + nullptr)}; if (state) { const Properties::const_iterator l = props.find(_uris.rdfs_label); if (l != props.end() && l->second.type() == _uris.atom_String) { - lilv_state_set_label(state, l->second.ptr()); + lilv_state_set_label(state.get(), l->second.ptr()); } - lilv_state_save(lworld, lmap, lunmap, state, nullptr, + lilv_state_save(lworld, lmap, lunmap, state.get(), nullptr, dirname.c_str(), basename.c_str()); - const URI state_uri(lilv_node_as_uri(lilv_state_get_uri(state))); - const std::string label(lilv_state_get_label(state) - ? lilv_state_get_label(state) - : basename); - lilv_state_free(state); + const URI state_uri(lilv_node_as_uri(lilv_state_get_uri(state.get()))); + const std::string label(lilv_state_get_label(state.get()) + ? lilv_state_get_label(state.get()) + : basename); Resource preset(_uris, state_uri); preset.set_property(_uris.rdf_type, _uris.pset_Preset); diff --git a/src/server/LV2Block.hpp b/src/server/LV2Block.hpp index 7a0a86ac..ec9c5ac3 100644 --- a/src/server/LV2Block.hpp +++ b/src/server/LV2Block.hpp @@ -95,7 +95,7 @@ public: void run(RunContext& ctx) override; void post_process(RunContext& ctx) override; - LilvState* load_preset(const URI& uri) override; + StatePtr load_preset(const URI& uri) override; void apply_state(const std::unique_ptr& worker, const LilvState* state) override; @@ -108,7 +108,7 @@ public: const BufferRef& buf, SampleCount offset) override; - static LilvState* load_state(World& world, const FilePath& path); + static StatePtr load_state(World& world, const FilePath& path); protected: struct Instance : public Raul::Noncopyable { diff --git a/src/server/State.hpp b/src/server/State.hpp new file mode 100644 index 00000000..3b745bdb --- /dev/null +++ b/src/server/State.hpp @@ -0,0 +1,33 @@ +/* + This file is part of Ingen. + Copyright 2020 David Robillard + + Ingen is free software: you can redistribute it and/or modify it under the + terms of the GNU Affero General Public License as published by the Free + Software Foundation, either version 3 of the License, or any later version. + + Ingen is distributed in the hope that it will be useful, but WITHOUT ANY + WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR + A PARTICULAR PURPOSE. See the GNU Affero General Public License for details. + + You should have received a copy of the GNU Affero General Public License + along with Ingen. If not, see . +*/ + +#ifndef INGEN_ENGINE_STATE_HPP +#define INGEN_ENGINE_STATE_HPP + +namespace ingen { +namespace server { + +struct StateDeleter +{ + void operator()(LilvState* state) { lilv_state_free(state); }; +}; + +using StatePtr = std::unique_ptr; + +} // namespace server +} // namespace ingen + +#endif // INGEN_ENGINE_STATE_HPP diff --git a/src/server/events/CreateBlock.cpp b/src/server/events/CreateBlock.cpp index 712a9966..4dc64bdc 100644 --- a/src/server/events/CreateBlock.cpp +++ b/src/server/events/CreateBlock.cpp @@ -134,7 +134,7 @@ CreateBlock::pre_process(PreProcessContext& ctx) } // Load state from directory if given in properties - LilvState* state = nullptr; + StatePtr state{}; auto s = _properties.find(uris.state_state); if (s != _properties.end() && s->second.type() == uris.forge.Path) { state = LV2Block::load_state( @@ -147,7 +147,7 @@ CreateBlock::pre_process(PreProcessContext& ctx) polyphonic, _graph, _engine, - state))) { + state.get()))) { return Event::pre_process_done(Status::CREATION_FAILED, _path); } } diff --git a/src/server/events/Delta.cpp b/src/server/events/Delta.cpp index 33611b7a..911ace66 100644 --- a/src/server/events/Delta.cpp +++ b/src/server/events/Delta.cpp @@ -72,7 +72,7 @@ Delta::Delta(Engine& engine, , _object(nullptr) , _graph(nullptr) , _binding(nullptr) - , _state(nullptr) + , _state() , _context(msg.ctx) , _type(Type::PUT) , _block(false) @@ -390,8 +390,9 @@ Delta::pre_process(PreProcessContext& ctx) if (!uri.empty()) { op = SpecialType::PRESET; if ((_state = block->load_preset(uri))) { - lilv_state_emit_port_values( - _state, s_add_set_event, this); + lilv_state_emit_port_values(_state.get(), + s_add_set_event, + this); } else { _engine.log().warn("Failed to load preset <%1%>\n", uri); } @@ -597,10 +598,11 @@ Delta::post_process() if (_state) { auto* block = dynamic_cast(_object); if (block) { - block->apply_state(_engine.sync_worker(), _state); + block->apply_state(_engine.sync_worker(), _state.get()); block->set_enabled(true); } - lilv_state_free(_state); + + _state.reset(); } Broadcaster::Transfer t(*_engine.broadcaster()); diff --git a/src/server/events/Delta.hpp b/src/server/events/Delta.hpp index 4d749f02..fc4716f2 100644 --- a/src/server/events/Delta.hpp +++ b/src/server/events/Delta.hpp @@ -17,6 +17,7 @@ #ifndef INGEN_EVENTS_DELTA_HPP #define INGEN_EVENTS_DELTA_HPP +#include "BlockImpl.hpp" #include "ClientUpdate.hpp" #include "CompiledGraph.hpp" #include "ControlBindings.hpp" @@ -119,7 +120,7 @@ private: GraphImpl* _graph; Raul::managed_ptr _compiled_graph; ControlBindings::Binding* _binding; - LilvState* _state; + StatePtr _state; Resource::Graph _context; Type _type; -- cgit v1.2.1