diff --git a/src/neuron/container/soa_identifier.hpp b/src/neuron/container/soa_identifier.hpp index 8ef76c8158..f4e7b105dc 100644 --- a/src/neuron/container/soa_identifier.hpp +++ b/src/neuron/container/soa_identifier.hpp @@ -84,29 +84,47 @@ struct owning_identifier { : owning_identifier(storage.acquire_owning_identifier()) {} owning_identifier(const owning_identifier&) = delete; - owning_identifier(owning_identifier&& other) = default; + owning_identifier(owning_identifier&& other) { + m_ptr = std::move(other.m_ptr); + other.m_ptr = nullptr; + + m_data_ptr = other.m_data_ptr; + other.m_data_ptr = nullptr; + } owning_identifier& operator=(const owning_identifier&) = delete; - owning_identifier& operator=(owning_identifier&&) = default; + owning_identifier& operator=(owning_identifier&& other) { + destroy(); + + m_ptr = std::move(other.m_ptr); + other.m_ptr = nullptr; + + m_data_ptr = other.m_data_ptr; + other.m_data_ptr = nullptr; + + return *this; + } - ~owning_identifier() = default; + ~owning_identifier() { + destroy(); + } /** * @brief Return a reference to the container in which this entry lives. */ Storage& underlying_storage() { - return *m_ptr.get_deleter().m_data_ptr; + return *m_data_ptr; } /** * @brief Return a const reference to the container in which this entry lives. */ Storage const& underlying_storage() const { - return *m_ptr.get_deleter().m_data_ptr; + return *m_data_ptr; } [[nodiscard]] operator non_owning_identifier() const { - return {const_cast(&underlying_storage()), m_ptr.get()}; + return {m_data_ptr, m_ptr}; } [[nodiscard]] operator non_owning_identifier_without_container() const { @@ -132,58 +150,67 @@ struct owning_identifier { private: owning_identifier() = default; - struct deleter { - deleter() = default; - deleter(Storage& data_container) - : m_data_ptr{&data_container} {} - void operator()(std::size_t* p) const { - assert(m_data_ptr); - auto& data_container = *m_data_ptr; - assert(p); - // We should still be a valid reference at this point. - assert(*p < data_container.size()); - // Prove that the bookkeeping works. - assert(data_container.at(*p) == p); - bool terminate{false}; - // Delete the corresponding row from `data_container` - try { - data_container.erase(*p); - } catch (std::exception const& e) { - // Cannot throw from unique_ptr release/reset/destructor, this - // is the best we can do. Most likely what has happened is - // something like: - // auto const read_only_token = node_data.issue_frozen_token(); - // list_of_nodes.pop_back(); - // which tries to delete a row from a container in read-only mode. - std::cerr << "neuron::container::owning_identifier<" - << cxx_demangle(typeid(Storage).name()) - << "> destructor could not delete from the underlying storage: " - << e.what() << " [" << cxx_demangle(typeid(e).name()) - << "]. This is not recoverable, aborting." << std::endl; - terminate = true; - } - if (terminate) { - std::terminate(); - } - // We don't know how many people know the pointer `p`, so write a sentinel - // value to it and transfer ownership "elsewhere". - *p = invalid_row; - // This is to provide compatibility with NEURON's old nrn_notify_when_double_freed and - // nrn_notify_when_void_freed methods. - detail::notify_handle_dying(p); - // This is sort-of formalising a memory leak. In principle we could cleanup - // identifier_defer_delete_storage by scanning all our data structures and finding - // references to the pointers that it contains. In practice it seems unlikely that - // either this, or using std::shared_ptr just to avoid it, would be worth it. - if (detail::identifier_defer_delete_storage) { - detail::identifier_defer_delete_storage->emplace_back(p); - } else { - delete p; - } + + void destroy() { + if (!m_ptr) { + // Nothing to be done. + return; + } + + assert(m_data_ptr); + auto& data_container = *m_data_ptr; + + // We should still be a valid reference at this point. + assert(m_ptr); + assert(*m_ptr < data_container.size()); + + // Prove that the bookkeeping works. + assert(data_container.at(*m_ptr) == m_ptr); + + bool terminate = false; + // Delete the corresponding row from `data_container` + try { + data_container.erase(*m_ptr); + } catch (std::exception const& e) { + // Cannot throw from unique_ptr release/reset/destructor, this + // is the best we can do. Most likely what has happened is + // something like: + // auto const read_only_token = node_data.issue_frozen_token(); + // list_of_nodes.pop_back(); + // which tries to delete a row from a container in read-only mode. + std::cerr << "neuron::container::owning_identifier<" + << cxx_demangle(typeid(Storage).name()) + << "> destructor could not delete from the underlying storage: " << e.what() + << " [" << cxx_demangle(typeid(e).name()) + << "]. This is not recoverable, aborting." << std::endl; + terminate = true; + } + if (terminate) { + std::terminate(); } - Storage* m_data_ptr{}; - }; - std::unique_ptr m_ptr; + // We don't know how many people know the pointer `p`, so write a sentinel + // value to it and transfer ownership "elsewhere". + set_current_row(invalid_row); + + // This is to provide compatibility with NEURON's old nrn_notify_when_double_freed and + // nrn_notify_when_void_freed methods. + detail::notify_handle_dying(m_ptr); + + // This is sort-of formalising a memory leak. In principle we could cleanup + // identifier_defer_delete_storage by scanning all our data structures and finding + // references to the pointers that it contains. In practice it seems unlikely that + // either this, or using std::shared_ptr just to avoid it, would be worth it. + if (detail::identifier_defer_delete_storage) { + detail::identifier_defer_delete_storage->emplace_back(m_ptr); + } else { + delete m_ptr; + } + } + + + size_t* m_ptr = nullptr; + Storage* m_data_ptr{}; + template friend struct soa; void set_current_row(std::size_t new_row) { @@ -198,8 +225,9 @@ struct owning_identifier { * be used without great care. */ owning_identifier(Storage& storage, std::size_t row) - : m_ptr{new std::size_t, storage} { - *m_ptr = row; - } + : m_ptr(new size_t) + , m_data_ptr(&storage) { + *m_ptr = row; + } }; } // namespace neuron::container