From 7fa73c5462c1fcc264d4ab8f271ec37fbd57378c Mon Sep 17 00:00:00 2001 From: Alexey Milovidov Date: Fri, 20 Jan 2017 06:33:56 +0300 Subject: [PATCH] Field: fixed memory leak [#CLICKHOUSE-2756]. --- dbms/include/DB/Core/Field.h | 150 +++++++++++++++++++++++++---------- 1 file changed, 106 insertions(+), 44 deletions(-) diff --git a/dbms/include/DB/Core/Field.h b/dbms/include/DB/Core/Field.h index e001cbb269f..65d991553d1 100644 --- a/dbms/include/DB/Core/Field.h +++ b/dbms/include/DB/Core/Field.h @@ -79,6 +79,7 @@ public: } }; + /// Позволяет получить идентификатор для типа или наоборот. template struct TypeToEnum; template struct EnumToType; @@ -99,7 +100,7 @@ public: Field(Field && rhs) { - move(std::move(rhs)); + create(std::move(rhs)); } template @@ -108,6 +109,12 @@ public: create(rhs); } + template + Field(T && rhs) + { + create(std::move(rhs)); + } + /// Создать строку inplace. Field(const char * data, size_t size) { @@ -119,6 +126,7 @@ public: create(data, size); } + /// NOTE In case when field already has string type, more direct assign is possible. void assignString(const char * data, size_t size) { destroy(); @@ -135,8 +143,13 @@ public: { if (this != &rhs) { - destroy(); - create(rhs); + if (which != rhs.which) + { + destroy(); + create(rhs); + } + else + assign(rhs); /// This assigns string or vector without deallocation of existing buffer. } return *this; } @@ -145,7 +158,13 @@ public: { if (this != &rhs) { - move(std::move(rhs)); + if (which != rhs.which) + { + destroy(); + create(std::move(rhs)); + } + else + assign(std::move(rhs)); } return *this; } @@ -153,8 +172,28 @@ public: template Field & operator= (const T & rhs) { - destroy(); - create(rhs); + if (which != TypeToEnum::value) + { + destroy(); + create(rhs); + } + else + assign(rhs); + + return *this; + } + + template + Field & operator= (T && rhs) + { + if (which != TypeToEnum::value) + { + destroy(); + create(std::move(rhs)); + } + else + assign(std::move(rhs)); + return *this; } @@ -289,6 +328,7 @@ private: Types::Which which; + /// Assuming there was no allocated state or it was deallocated (see destroy). template void create(const T & x) { @@ -297,25 +337,70 @@ private: new (ptr) T(x); } - void create(const Null & x) + template + void create(T && x) { - which = Types::Null; + which = TypeToEnum::value; + T * __attribute__((__may_alias__)) ptr = reinterpret_cast(storage); + new (ptr) T(std::move(x)); } + /// Assuming same types. + template + void assign(const T & x) + { + T * __attribute__((__may_alias__)) ptr = reinterpret_cast(storage); + *ptr = x; + } + + template + void assign(T && x) + { + T * __attribute__((__may_alias__)) ptr = reinterpret_cast(storage); + *ptr = std::move(x); + } + + + template /// Field template parameter may be const or non-const Field. + static void dispatch(F && f, Field & field) + { + switch (field.which) + { + case Types::Null: f(field.get()); return; + case Types::UInt64: f(field.get()); return; + case Types::Int64: f(field.get()); return; + case Types::Float64: f(field.get()); return; + case Types::String: f(field.get()); return; + case Types::Array: f(field.get()); return; + case Types::Tuple: f(field.get()); return; + + default: + throw Exception("Bad type of Field", ErrorCodes::BAD_TYPE_OF_FIELD); + } + } + + void create(const Field & x) { - switch (x.which) - { - case Types::Null: create(Null()); break; - case Types::UInt64: create(x.get()); break; - case Types::Int64: create(x.get()); break; - case Types::Float64: create(x.get()); break; - case Types::String: create(x.get()); break; - case Types::Array: create(x.get()); break; - case Types::Tuple: create(x.get()); break; - } + dispatch([this] (auto & value) { create(value); }, x); } + void create(Field && x) + { + dispatch([this] (auto & value) { create(std::move(value)); }, x); + } + + void assign(const Field & x) + { + dispatch([this] (auto & value) { assign(value); }, x); + } + + void assign(Field && x) + { + dispatch([this] (auto & value) { assign(std::move(value)); }, x); + } + + void create(const char * data, size_t size) { which = Types::String; @@ -347,6 +432,8 @@ private: default: break; } + + which = Types::Null; /// for exception safety in subsequent calls to destroy and create, when create fails. } template @@ -355,31 +442,6 @@ private: T * __attribute__((__may_alias__)) ptr = reinterpret_cast(storage); ptr->~T(); } - - template - void moveValue(Field && x) - { - T * __attribute__((__may_alias__)) ptr_this = reinterpret_cast(storage); - T * __attribute__((__may_alias__)) ptr_x = reinterpret_cast(x.storage); - - new (ptr_this) T(std::move(*ptr_x)); - } - - void move(Field && x) - { - which = x.which; - - switch (x.which) - { - case Types::Null: create(Null()); break; - case Types::UInt64: create(x.get()); break; - case Types::Int64: create(x.get()); break; - case Types::Float64: create(x.get()); break; - case Types::String: moveValue(std::move(x)); break; - case Types::Array: moveValue(std::move(x)); break; - case Types::Tuple: moveValue(std::move(x)); break; - } - } }; #undef DBMS_MIN_FIELD_SIZE @@ -393,7 +455,7 @@ template <> struct Field::TypeToEnum { static const Types::Which template <> struct Field::TypeToEnum { static const Types::Which value = Types::Array; }; template <> struct Field::TypeToEnum { static const Types::Which value = Types::Tuple; }; -template <> struct Field::EnumToType { using Type = Null ; }; +template <> struct Field::EnumToType { using Type = Null ; }; template <> struct Field::EnumToType { using Type = UInt64 ; }; template <> struct Field::EnumToType { using Type = Int64 ; }; template <> struct Field::EnumToType { using Type = Float64 ; };