From d12fe4a7d2fa23b49659925540e52de153a5363f Mon Sep 17 00:00:00 2001 From: Liqf <109049295+LemonLiTree@users.noreply.github.com> Date: Thu, 27 Apr 2023 09:04:10 +0800 Subject: [PATCH] [bug](fix)fix Geo memory leak (#19116) --- be/src/geo/geo_tobinary.cpp | 2 +- be/src/geo/geo_types.cpp | 16 ++++++++-------- be/src/geo/geo_types.h | 11 ++++++++++- be/src/geo/wkb_parse.cpp | 24 ++++++++++++------------ be/src/geo/wkb_parse.h | 9 +++++---- be/src/geo/wkt_yacc.y | 6 +++--- 6 files changed, 39 insertions(+), 29 deletions(-) diff --git a/be/src/geo/geo_tobinary.cpp b/be/src/geo/geo_tobinary.cpp index 46ffc1b0f0..0e64c00168 100644 --- a/be/src/geo/geo_tobinary.cpp +++ b/be/src/geo/geo_tobinary.cpp @@ -88,7 +88,7 @@ bool toBinary::writeGeoPolygon(doris::GeoPolygon* polygon, ToBinaryContext* ctx) writeByteOrder(ctx); writeGeometryType(wkbType::wkbPolygon, ctx); writeInt(polygon->numLoops(), ctx); - GeoCoordinateListList* coordss = polygon->to_coords(); + std::unique_ptr coordss(polygon->to_coords()); for (int i = 0; i < coordss->list.size(); ++i) { writeCoordinateList(*coordss->list[i], true, ctx); diff --git a/be/src/geo/geo_types.cpp b/be/src/geo/geo_types.cpp index c134dbfb6e..ec06ffdfa2 100644 --- a/be/src/geo/geo_types.cpp +++ b/be/src/geo/geo_types.cpp @@ -219,19 +219,19 @@ GeoShape* GeoShape::from_encoded(const void* ptr, size_t size) { std::unique_ptr shape; switch (((const char*)ptr)[1]) { case GEO_SHAPE_POINT: { - shape.reset(new GeoPoint()); + shape.reset(GeoPoint::create_unique().release()); break; } case GEO_SHAPE_LINE_STRING: { - shape.reset(new GeoLine()); + shape.reset(GeoLine::create_unique().release()); break; } case GEO_SHAPE_POLYGON: { - shape.reset(new GeoPolygon()); + shape.reset(GeoPolygon::create_unique().release()); break; } case GEO_SHAPE_CIRCLE: { - shape.reset(new GeoCircle()); + shape.reset(GeoCircle::create_unique().release()); break; } default: @@ -274,10 +274,10 @@ GeoCoordinateList GeoLine::to_coords() const { return coords; } -GeoCoordinateListList* GeoPolygon::to_coords() const { - GeoCoordinateListList* coordss = new GeoCoordinateListList(); +const std::unique_ptr GeoPolygon::to_coords() const { + std::unique_ptr coordss(new GeoCoordinateListList()); for (int i = 0; i < GeoPolygon::numLoops(); ++i) { - GeoCoordinateList* coords = new GeoCoordinateList(); + std::unique_ptr coords(new GeoCoordinateList()); S2Loop* loop = GeoPolygon::getLoop(i); for (int j = 0; j < loop->num_vertices(); ++j) { GeoCoordinate coord; @@ -294,7 +294,7 @@ GeoCoordinateListList* GeoPolygon::to_coords() const { coords->add(coord); } } - coordss->add(coords); + coordss->add(coords.release()); } return coordss; } diff --git a/be/src/geo/geo_types.h b/be/src/geo/geo_types.h index 9bba6140b8..aecd9bbd53 100644 --- a/be/src/geo/geo_types.h +++ b/be/src/geo/geo_types.h @@ -22,6 +22,7 @@ #include #include +#include "common/factory_creator.h" #include "geo/geo_common.h" #include "geo/wkt_parse_type.h" @@ -70,6 +71,8 @@ protected: }; class GeoPoint : public GeoShape { + ENABLE_FACTORY_CREATOR(GeoPoint); + public: GeoPoint(); ~GeoPoint() override; @@ -106,6 +109,8 @@ private: }; class GeoLine : public GeoShape { + ENABLE_FACTORY_CREATOR(GeoLine); + public: GeoLine(); ~GeoLine() override; @@ -131,12 +136,14 @@ private: }; class GeoPolygon : public GeoShape { + ENABLE_FACTORY_CREATOR(GeoPolygon); + public: GeoPolygon(); ~GeoPolygon() override; GeoParseStatus from_coords(const GeoCoordinateListList& list); - GeoCoordinateListList* to_coords() const; + const std::unique_ptr to_coords() const; GeoShapeType type() const override { return GEO_SHAPE_POLYGON; } const S2Polygon* polygon() const { return _polygon.get(); } @@ -157,6 +164,8 @@ private: }; class GeoCircle : public GeoShape { + ENABLE_FACTORY_CREATOR(GeoCircle); + public: GeoCircle(); ~GeoCircle() override; diff --git a/be/src/geo/wkb_parse.cpp b/be/src/geo/wkb_parse.cpp index 2dafc1bbc7..e24328d756 100644 --- a/be/src/geo/wkb_parse.cpp +++ b/be/src/geo/wkb_parse.cpp @@ -127,7 +127,7 @@ WkbParseContext* WkbParse::read(std::istream& is, WkbParseContext* ctx) { ctx->dis = ByteOrderDataInStream(buf.data(), buf.size()); // will default to machine endian - ctx->shape = readGeometry(ctx); + ctx->shape = readGeometry(ctx).release(); if (!ctx->shape) { ctx->parse_status = GEO_PARSE_WKB_SYNTAX_ERROR; @@ -135,7 +135,7 @@ WkbParseContext* WkbParse::read(std::istream& is, WkbParseContext* ctx) { return ctx; } -GeoShape* WkbParse::readGeometry(WkbParseContext* ctx) { +std::unique_ptr WkbParse::readGeometry(WkbParseContext* ctx) { // determine byte order unsigned char byteOrder = ctx->dis.readByte(); @@ -150,17 +150,17 @@ GeoShape* WkbParse::readGeometry(WkbParseContext* ctx) { uint32_t geometryType = (typeInt & 0xffff) % 1000; - GeoShape* shape; + std::unique_ptr shape; switch (geometryType) { case wkbType::wkbPoint: - shape = readPoint(ctx); + shape.reset(readPoint(ctx).release()); break; case wkbType::wkbLine: - shape = readLine(ctx); + shape.reset(readLine(ctx).release()); break; case wkbType::wkbPolygon: - shape = readPolygon(ctx); + shape.reset(readPolygon(ctx).release()); break; default: return nullptr; @@ -168,9 +168,9 @@ GeoShape* WkbParse::readGeometry(WkbParseContext* ctx) { return shape; } -GeoPoint* WkbParse::readPoint(WkbParseContext* ctx) { +std::unique_ptr WkbParse::readPoint(WkbParseContext* ctx) { GeoCoordinateList coords = WkbParse::readCoordinateList(1, ctx); - GeoPoint* point = new GeoPoint(); + std::unique_ptr point = GeoPoint::create_unique(); if (point->from_coord(coords.list[0]) == GEO_PARSE_OK) { return point; @@ -179,12 +179,12 @@ GeoPoint* WkbParse::readPoint(WkbParseContext* ctx) { } } -GeoLine* WkbParse::readLine(WkbParseContext* ctx) { +std::unique_ptr WkbParse::readLine(WkbParseContext* ctx) { uint32_t size = ctx->dis.readUnsigned(); minMemSize(wkbLine, size, ctx); GeoCoordinateList coords = WkbParse::readCoordinateList(size, ctx); - GeoLine* line = new GeoLine(); + std::unique_ptr line = GeoLine::create_unique(); if (line->from_coords(coords) == GEO_PARSE_OK) { return line; @@ -193,7 +193,7 @@ GeoLine* WkbParse::readLine(WkbParseContext* ctx) { } } -GeoPolygon* WkbParse::readPolygon(WkbParseContext* ctx) { +std::unique_ptr WkbParse::readPolygon(WkbParseContext* ctx) { uint32_t num_loops = ctx->dis.readUnsigned(); minMemSize(wkbPolygon, num_loops, ctx); GeoCoordinateListList coordss; @@ -204,7 +204,7 @@ GeoPolygon* WkbParse::readPolygon(WkbParseContext* ctx) { coordss.add(coords); } - GeoPolygon* polygon = new GeoPolygon(); + std::unique_ptr polygon = GeoPolygon::create_unique(); if (polygon->from_coords(coordss) == GEO_PARSE_OK) { return polygon; diff --git a/be/src/geo/wkb_parse.h b/be/src/geo/wkb_parse.h index f062a5ab75..e03dddf97a 100644 --- a/be/src/geo/wkb_parse.h +++ b/be/src/geo/wkb_parse.h @@ -20,6 +20,7 @@ #include #include +#include #include "geo/geo_common.h" #include "geo/wkt_parse_type.h" @@ -41,14 +42,14 @@ public: static WkbParseContext* read(std::istream& is, WkbParseContext* ctx); - static GeoShape* readGeometry(WkbParseContext* ctx); + static std::unique_ptr readGeometry(WkbParseContext* ctx); private: - static GeoPoint* readPoint(WkbParseContext* ctx); + static std::unique_ptr readPoint(WkbParseContext* ctx); - static GeoLine* readLine(WkbParseContext* ctx); + static std::unique_ptr readLine(WkbParseContext* ctx); - static GeoPolygon* readPolygon(WkbParseContext* ctx); + static std::unique_ptr readPolygon(WkbParseContext* ctx); static GeoCoordinateList readCoordinateList(unsigned size, WkbParseContext* ctx); diff --git a/be/src/geo/wkt_yacc.y b/be/src/geo/wkt_yacc.y index 7757cbef36..1af26e82ee 100644 --- a/be/src/geo/wkt_yacc.y +++ b/be/src/geo/wkt_yacc.y @@ -92,7 +92,7 @@ shape: point: KW_POINT '(' coordinate ')' { - std::unique_ptr point(new doris::GeoPoint()); + std::unique_ptr point = doris::GeoPoint::create_unique(); ctx->parse_status = point->from_coord($3); if (ctx->parse_status != doris::GEO_PARSE_OK) { YYABORT; @@ -106,7 +106,7 @@ linestring: { // to avoid memory leak std::unique_ptr list($3); - std::unique_ptr line(new doris::GeoLine()); + std::unique_ptr line = doris::GeoLine::create_unique(); ctx->parse_status = line->from_coords(*$3); if (ctx->parse_status != doris::GEO_PARSE_OK) { YYABORT; @@ -120,7 +120,7 @@ polygon: { // to avoid memory leak std::unique_ptr list($3); - std::unique_ptr polygon(new doris::GeoPolygon()); + std::unique_ptr polygon = doris::GeoPolygon::create_unique(); ctx->parse_status = polygon->from_coords(*$3); if (ctx->parse_status != doris::GEO_PARSE_OK) { YYABORT;