diff --git a/userspace/libsinsp/state/dynamic_struct.h b/userspace/libsinsp/state/dynamic_struct.h index 7ee5cf1dec..6705f7d094 100644 --- a/userspace/libsinsp/state/dynamic_struct.h +++ b/userspace/libsinsp/state/dynamic_struct.h @@ -267,21 +267,26 @@ class dynamic_struct }; inline explicit dynamic_struct(const std::shared_ptr& dynamic_fields) - : m_fields_len(0), m_fields(), m_dynamic_fields(dynamic_fields) { } + : m_fields(), m_dynamic_fields(dynamic_fields) { } + inline dynamic_struct(dynamic_struct&&) = default; - inline dynamic_struct& operator = (dynamic_struct&&) = default; - inline dynamic_struct(const dynamic_struct& s) = default; - inline dynamic_struct& operator = (const dynamic_struct& s) = default; + + inline dynamic_struct& operator=(dynamic_struct&&) = default; + + inline dynamic_struct(const dynamic_struct& s) + { + deep_fields_copy(s); + } + + inline dynamic_struct& operator=(const dynamic_struct& s) + { + deep_fields_copy(s); + return *this; + } + virtual ~dynamic_struct() { - if (m_dynamic_fields) - { - for (size_t i = 0; i < m_fields.size(); i++) - { - m_dynamic_fields->m_definitions_ordered[i]->info().destroy(m_fields[i]); - free(m_fields[i]); - } - } + destroy_dynamic_fields(); } /** @@ -323,6 +328,10 @@ class dynamic_struct */ virtual void set_dynamic_fields(const std::shared_ptr& defs) { + if (m_dynamic_fields.get() == defs.get()) + { + return; + } if (m_dynamic_fields && m_dynamic_fields.use_count() > 1) { throw sinsp_exception("dynamic struct defintions set twice"); @@ -373,6 +382,23 @@ class dynamic_struct } } + /** + * @brief Destroys all the dynamic field values currently allocated + */ + virtual void destroy_dynamic_fields() + { + if (!m_dynamic_fields) + { + return; + } + for (size_t i = 0; i < m_fields.size(); i++) + { + m_dynamic_fields->m_definitions_ordered[i]->info().destroy(m_fields[i]); + free(m_fields[i]); + } + m_fields.clear(); + } + private: inline void _check_defsptr(const field_info& i, bool write) const { @@ -400,18 +426,38 @@ class dynamic_struct { throw sinsp_exception("dynamic struct access overflow: " + std::to_string(index)); } - while (m_fields_len <= index) + while (m_fields.size() <= index) { - auto def = m_dynamic_fields->m_definitions_ordered[m_fields_len]; + auto def = m_dynamic_fields->m_definitions_ordered[m_fields.size()]; void* fieldbuf = malloc(def->info().size()); def->info().construct(fieldbuf); m_fields.push_back(fieldbuf); - m_fields_len++; } return m_fields[index]; } - size_t m_fields_len; + inline void deep_fields_copy(const dynamic_struct& other_const) + { + // note: const cast should be safe here as we're not going to resize + // nor edit the dynamic fields allocated in "other" + auto& other = const_cast(other_const); + + // copy the definitions + set_dynamic_fields(other.dynamic_fields()); + + // deep copy of all the fields + destroy_dynamic_fields(); + for (size_t i = 0; i < other.m_fields.size(); i++) + { + const auto info = m_dynamic_fields->m_definitions_ordered[i]; + // note: we use uintptr_t as it fits all the data types supported for + // reading and writing dynamic fields (e.g. uint32_t, uint64_t, const char*, base_table*, ...) + uintptr_t val = 0; + other.get_dynamic_field(*info, reinterpret_cast(&val)); + set_dynamic_field(*info, &val); + } + } + std::vector m_fields; std::shared_ptr m_dynamic_fields; }; diff --git a/userspace/libsinsp/state/table.h b/userspace/libsinsp/state/table.h index 87c00d15e5..8c13f78c0d 100644 --- a/userspace/libsinsp/state/table.h +++ b/userspace/libsinsp/state/table.h @@ -112,6 +112,10 @@ class base_table virtual void set_dynamic_fields(const std::shared_ptr& dynf) { + if (m_dynamic_fields.get() == dynf.get()) + { + return; + } if (!dynf) { throw sinsp_exception("null definitions passed to set_dynamic_fields"); diff --git a/userspace/libsinsp/state/table_adapters.h b/userspace/libsinsp/state/table_adapters.h index 98962bacfd..0580166370 100644 --- a/userspace/libsinsp/state/table_adapters.h +++ b/userspace/libsinsp/state/table_adapters.h @@ -127,6 +127,11 @@ template class value_table_entry_adapter : public libsinsp::state::t } } + virtual void destroy_dynamic_fields() override final + { + // nothing to do + } + private: T* m_value; }; diff --git a/userspace/libsinsp/test/state.ut.cpp b/userspace/libsinsp/test/state.ut.cpp index ac5350b245..ea2c15ce7a 100644 --- a/userspace/libsinsp/test/state.ut.cpp +++ b/userspace/libsinsp/test/state.ut.cpp @@ -161,7 +161,7 @@ TEST(dynamic_struct, defs_and_access) // struct construction and setting fields definition sample_struct s(fields); ASSERT_ANY_THROW(s.set_dynamic_fields(nullptr)); - ASSERT_ANY_THROW(s.set_dynamic_fields(fields)); + ASSERT_ANY_THROW(s.set_dynamic_fields(std::make_shared())); // The double paranthesis fixes // Error C2063 'std::shared_ptr' : not a function C // on the Windows compiler. @@ -170,7 +170,7 @@ TEST(dynamic_struct, defs_and_access) ASSERT_NO_THROW(sample_struct(nullptr)); auto s2 = sample_struct(nullptr); s2.set_dynamic_fields(fields); - ASSERT_ANY_THROW(s2.set_dynamic_fields(fields)); + ASSERT_NO_THROW(s2.set_dynamic_fields(fields)); // check field definitions ASSERT_EQ(fields->fields().size(), 0); @@ -230,6 +230,77 @@ TEST(dynamic_struct, defs_and_access) ASSERT_ANY_THROW(s.get_dynamic_field(acc_num2, tmp)); } +TEST(dynamic_struct, mem_ownership) +{ + struct sample_struct: public libsinsp::state::dynamic_struct + { + sample_struct(const std::shared_ptr& i): dynamic_struct(i) { } + }; + + std::string tmpstr1, tmpstr2; + auto defs1 = std::make_shared(); + + // construct two entries, test safety checks + sample_struct s1(nullptr); + ASSERT_NO_THROW(s1.set_dynamic_fields(nullptr)); + ASSERT_NO_THROW(s1.set_dynamic_fields(defs1)); + sample_struct s2(defs1); + ASSERT_ANY_THROW(s1.set_dynamic_fields(nullptr)); + ASSERT_NO_THROW(s1.set_dynamic_fields(defs1)); + ASSERT_ANY_THROW(s1.set_dynamic_fields(std::make_shared())); + + // define a string dynamic field + auto field_str = defs1->add_field("str"); + auto field_str_acc = field_str.new_accessor(); + + // write same value in both structs, ensure they have two distinct copies + s1.set_dynamic_field(field_str_acc, std::string("hello")); + s1.get_dynamic_field(field_str_acc, tmpstr1); + ASSERT_EQ(tmpstr1, std::string("hello")); + s2.get_dynamic_field(field_str_acc, tmpstr2); + ASSERT_EQ(tmpstr2, std::string("")); // s2 should not be influenced + s2.set_dynamic_field(field_str_acc, std::string("hello2")); + s2.get_dynamic_field(field_str_acc, tmpstr2); + ASSERT_EQ(tmpstr2, tmpstr1 + "2"); + s1.get_dynamic_field(field_str_acc, tmpstr1); // s1 should not be influenced + ASSERT_EQ(tmpstr2, tmpstr1 + "2"); + + // deep copy and memory ownership (constructor) + sample_struct s3(s1); + ASSERT_EQ(s1.dynamic_fields().get(), s3.dynamic_fields().get()); + s1.get_dynamic_field(field_str_acc, tmpstr1); + s3.get_dynamic_field(field_str_acc, tmpstr2); + ASSERT_EQ(tmpstr1, tmpstr2); + s3.set_dynamic_field(field_str_acc, std::string("hello3")); + s1.get_dynamic_field(field_str_acc, tmpstr1); // should still be "hello" as before + s3.get_dynamic_field(field_str_acc, tmpstr2); + ASSERT_NE(tmpstr1, tmpstr2); + + // deep copy and memory ownership (assignment) + sample_struct s4(std::make_shared()); + s4 = s1; + ASSERT_EQ(s1.dynamic_fields().get(), s4.dynamic_fields().get()); + s1.get_dynamic_field(field_str_acc, tmpstr1); + s4.get_dynamic_field(field_str_acc, tmpstr2); + ASSERT_EQ(tmpstr1, tmpstr2); + s4.set_dynamic_field(field_str_acc, std::string("hello4")); + s1.get_dynamic_field(field_str_acc, tmpstr1); // should still be "hello" as before + s4.get_dynamic_field(field_str_acc, tmpstr2); + ASSERT_NE(tmpstr1, tmpstr2); + + // deep copy and memory ownership (assignment, null initial definitions) + sample_struct s5(nullptr); + s5 = s1; + ASSERT_EQ(s1.dynamic_fields().get(), s5.dynamic_fields().get()); + s1.get_dynamic_field(field_str_acc, tmpstr1); + s5.get_dynamic_field(field_str_acc, tmpstr2); + ASSERT_EQ(tmpstr1, tmpstr2); + s5.set_dynamic_field(field_str_acc, std::string("hello4")); + s1.get_dynamic_field(field_str_acc, tmpstr1); // should still be "hello" as before + s5.get_dynamic_field(field_str_acc, tmpstr2); + ASSERT_NE(tmpstr1, tmpstr2); +} + TEST(table_registry, defs_and_access) { class sample_table: public libsinsp::state::table