Skip to content

Commit 3fcfd66

Browse files
github-actions[bot]linrrzqqq
github-actions[bot]
and
linrrzqqq
authored
branch-4.0: [Fix](geo) fix memory leak in geo #58004 (#58365)
Cherry-picked from #58004 Co-authored-by: linrrarity <[email protected]>
1 parent 1d964d8 commit 3fcfd66

File tree

16 files changed

+642
-454
lines changed

16 files changed

+642
-454
lines changed

‎be/src/geo/geo_types.cpp‎

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -403,13 +403,15 @@ void GeoShape::encode_to(std::string* buf) {
403403
encode(buf);
404404
}
405405

406-
GeoShape* GeoShape::from_wkt(const char* data, size_t size, GeoParseStatus* status) {
407-
GeoShape* shape = nullptr;
408-
*status = WktParse::parse_wkt(data, size, &shape);
406+
std::unique_ptr<GeoShape> GeoShape::from_wkt(const char* data, size_t size,
407+
GeoParseStatus& status) {
408+
std::unique_ptr<GeoShape> shape;
409+
status = WktParse::parse_wkt(data, size, shape);
409410
return shape;
410411
}
411412

412-
GeoShape* GeoShape::from_wkb(const char* data, size_t size, GeoParseStatus* status) {
413+
std::unique_ptr<GeoShape> GeoShape::from_wkb(const char* data, size_t size,
414+
GeoParseStatus& status) {
413415
std::stringstream wkb;
414416

415417
for (int i = 0; i < size; ++i) {
@@ -419,8 +421,8 @@ GeoShape* GeoShape::from_wkb(const char* data, size_t size, GeoParseStatus* stat
419421
wkb << *data;
420422
data++;
421423
}
422-
GeoShape* shape = nullptr;
423-
*status = WkbParse::parse_wkb(wkb, &shape);
424+
std::unique_ptr<GeoShape> shape;
425+
status = WkbParse::parse_wkb(wkb, shape);
424426
return shape;
425427
}
426428

@@ -490,7 +492,7 @@ GeoCoordinateList GeoLine::to_coords() const {
490492
return coords;
491493
}
492494

493-
const std::unique_ptr<GeoCoordinateListList> GeoPolygon::to_coords() const {
495+
std::unique_ptr<GeoCoordinateListList> GeoPolygon::to_coords() const {
494496
std::unique_ptr<GeoCoordinateListList> coordss(new GeoCoordinateListList());
495497
for (int i = 0; i < GeoPolygon::numLoops(); ++i) {
496498
std::unique_ptr<GeoCoordinateList> coords(new GeoCoordinateList());
@@ -510,12 +512,12 @@ const std::unique_ptr<GeoCoordinateListList> GeoPolygon::to_coords() const {
510512
coords->add(coord);
511513
}
512514
}
513-
coordss->add(coords.release());
515+
coordss->add(std::move(coords));
514516
}
515517
return coordss;
516518
}
517519

518-
const std::vector<std::unique_ptr<GeoCoordinateListList>> GeoMultiPolygon::to_coords() const {
520+
std::vector<std::unique_ptr<GeoCoordinateListList>> GeoMultiPolygon::to_coords() const {
519521
std::vector<std::unique_ptr<GeoCoordinateListList>> coordss;
520522
for (const auto& polygon : _polygons) {
521523
std::unique_ptr<GeoCoordinateListList> coords = polygon->to_coords();

‎be/src/geo/geo_types.h‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -51,9 +51,11 @@ class GeoShape {
5151
// try to construct a GeoShape from a WKT. If construct successfully, a GeoShape will
5252
// be returned, and the client should delete it when don't need it.
5353
// return nullptr if convert failed, and reason will be set in status
54-
static GeoShape* from_wkt(const char* data, size_t size, GeoParseStatus* status);
54+
static std::unique_ptr<GeoShape> from_wkt(const char* data, size_t size,
55+
GeoParseStatus& status);
5556

56-
static GeoShape* from_wkb(const char* data, size_t size, GeoParseStatus* status);
57+
static std::unique_ptr<GeoShape> from_wkb(const char* data, size_t size,
58+
GeoParseStatus& status);
5759

5860
void encode_to(std::string* buf);
5961
bool decode_from(const void* data, size_t size);
@@ -160,7 +162,7 @@ class GeoPolygon : public GeoShape {
160162
~GeoPolygon() override;
161163

162164
GeoParseStatus from_coords(const GeoCoordinateListList& list);
163-
const std::unique_ptr<GeoCoordinateListList> to_coords() const;
165+
std::unique_ptr<GeoCoordinateListList> to_coords() const;
164166

165167
GeoShapeType type() const override { return GEO_SHAPE_POLYGON; }
166168
const S2Polygon* polygon() const { return _polygon.get(); }
@@ -196,7 +198,7 @@ class GeoMultiPolygon : public GeoShape {
196198

197199
GeoParseStatus check_self_intersection();
198200
GeoParseStatus from_coords(const std::vector<GeoCoordinateListList>& list);
199-
const std::vector<std::unique_ptr<GeoCoordinateListList>> to_coords() const;
201+
std::vector<std::unique_ptr<GeoCoordinateListList>> to_coords() const;
200202

201203
GeoShapeType type() const override { return GEO_SHAPE_MULTI_POLYGON; }
202204
const std::vector<std::unique_ptr<GeoPolygon>>& polygons() const { return _polygons; }

‎be/src/geo/wkb_parse.cpp‎

Lines changed: 45 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
#include <cstddef>
2121
#include <istream>
2222
#include <sstream>
23+
#include <utility>
2324
#include <vector>
2425

2526
#include "geo/ByteOrderDataInStream.h"
@@ -75,19 +76,17 @@ unsigned char ASCIIHexToUChar(char val) {
7576
}
7677
}
7778

78-
GeoParseStatus WkbParse::parse_wkb(std::istream& is, GeoShape** shape) {
79+
GeoParseStatus WkbParse::parse_wkb(std::istream& is, std::unique_ptr<GeoShape>& shape) {
7980
WkbParseContext ctx;
8081

81-
ctx = *(WkbParse::read_hex(is, &ctx));
82+
WkbParse::read_hex(is, ctx);
8283
if (ctx.parse_status == GEO_PARSE_OK) {
83-
*shape = ctx.shape;
84-
} else {
85-
ctx.parse_status = GEO_PARSE_WKT_SYNTAX_ERROR;
84+
shape = std::move(ctx.shape);
8685
}
8786
return ctx.parse_status;
8887
}
8988

90-
WkbParseContext* WkbParse::read_hex(std::istream& is, WkbParseContext* ctx) {
89+
void WkbParse::read_hex(std::istream& is, WkbParseContext& ctx) {
9190
// setup input/output stream
9291
std::stringstream os(std::ios_base::binary | std::ios_base::in | std::ios_base::out);
9392

@@ -99,8 +98,8 @@ WkbParseContext* WkbParse::read_hex(std::istream& is, WkbParseContext* ctx) {
9998

10099
const int input_low = is.get();
101100
if (input_low == std::char_traits<char>::eof()) {
102-
ctx->parse_status = GEO_PARSE_WKB_SYNTAX_ERROR;
103-
return ctx;
101+
ctx.parse_status = GEO_PARSE_WKB_SYNTAX_ERROR;
102+
return;
104103
}
105104

106105
const char high = static_cast<char>(input_high);
@@ -109,80 +108,79 @@ WkbParseContext* WkbParse::read_hex(std::istream& is, WkbParseContext* ctx) {
109108
const unsigned char result_high = ASCIIHexToUChar(high);
110109
const unsigned char result_low = ASCIIHexToUChar(low);
111110

112-
const unsigned char value = static_cast<unsigned char>((result_high << 4) + result_low);
111+
const auto value = static_cast<unsigned char>((result_high << 4) + result_low);
113112

114113
// write the value to the output stream
115114
os << value;
116115
}
117-
return WkbParse::read(os, ctx);
116+
WkbParse::read(os, ctx);
118117
}
119118

120-
WkbParseContext* WkbParse::read(std::istream& is, WkbParseContext* ctx) {
119+
void WkbParse::read(std::istream& is, WkbParseContext& ctx) {
121120
is.seekg(0, std::ios::end);
122121
auto size = is.tellg();
123122
is.seekg(0, std::ios::beg);
124123

125124
// Check if size is valid
126125
if (size <= 0) {
127-
ctx->parse_status = GEO_PARSE_WKB_SYNTAX_ERROR;
128-
return ctx;
126+
ctx.parse_status = GEO_PARSE_WKB_SYNTAX_ERROR;
127+
return;
129128
}
130129

131130
std::vector<unsigned char> buf(static_cast<size_t>(size));
132131
if (!is.read(reinterpret_cast<char*>(buf.data()), static_cast<std::streamsize>(size))) {
133-
ctx->parse_status = GEO_PARSE_WKB_SYNTAX_ERROR;
134-
return ctx;
132+
ctx.parse_status = GEO_PARSE_WKB_SYNTAX_ERROR;
133+
return;
135134
}
136135

137136
// Ensure we have at least one byte for byte order
138137
if (buf.empty()) {
139-
ctx->parse_status = GEO_PARSE_WKB_SYNTAX_ERROR;
140-
return ctx;
138+
ctx.parse_status = GEO_PARSE_WKB_SYNTAX_ERROR;
139+
return;
141140
}
142141

143142
// First read the byte order using machine endian
144143
auto byteOrder = buf[0];
145144

146145
// Create ByteOrderDataInStream with the correct byte order
147146
if (byteOrder == byteOrder::wkbNDR) {
148-
ctx->dis = ByteOrderDataInStream(buf.data(), buf.size());
149-
ctx->dis.setOrder(ByteOrderValues::ENDIAN_LITTLE);
147+
ctx.dis = ByteOrderDataInStream(buf.data(), buf.size());
148+
ctx.dis.setOrder(ByteOrderValues::ENDIAN_LITTLE);
150149
} else if (byteOrder == byteOrder::wkbXDR) {
151-
ctx->dis = ByteOrderDataInStream(buf.data(), buf.size());
152-
ctx->dis.setOrder(ByteOrderValues::ENDIAN_BIG);
150+
ctx.dis = ByteOrderDataInStream(buf.data(), buf.size());
151+
ctx.dis.setOrder(ByteOrderValues::ENDIAN_BIG);
153152
} else {
154-
ctx->parse_status = GEO_PARSE_WKB_SYNTAX_ERROR;
155-
return ctx;
153+
ctx.parse_status = GEO_PARSE_WKB_SYNTAX_ERROR;
154+
return;
156155
}
157156

158157
std::unique_ptr<GeoShape> shape = readGeometry(ctx);
159158
if (!shape) {
160-
ctx->parse_status = GEO_PARSE_WKB_SYNTAX_ERROR;
161-
return ctx;
159+
ctx.parse_status = GEO_PARSE_WKB_SYNTAX_ERROR;
160+
return;
162161
}
163162

164-
ctx->shape = shape.release();
165-
return ctx;
163+
ctx.shape = std::move(shape);
166164
}
167165

168-
std::unique_ptr<GeoShape> WkbParse::readGeometry(WkbParseContext* ctx) {
166+
std::unique_ptr<GeoShape> WkbParse::readGeometry(WkbParseContext& ctx) {
169167
try {
170168
// Ensure we have enough data to read
171-
if (ctx->dis.size() < 5) { // At least 1 byte for order and 4 bytes for type
169+
if (ctx.dis.size() < 5) { // At least 1 byte for order and 4 bytes for type
172170
return nullptr;
173171
}
174172

175173
// Skip the byte order as we've already handled it
176-
ctx->dis.readByte();
174+
ctx.dis.readByte();
177175

178-
uint32_t typeInt = ctx->dis.readUnsigned();
176+
uint32_t typeInt = ctx.dis.readUnsigned();
179177

180178
// Check if geometry has SRID
181179
bool has_srid = (typeInt & WKB_SRID_FLAG) != 0;
182180

183181
// Read SRID if present
184182
if (has_srid) {
185-
ctx->dis.readUnsigned(); // Read and store SRID if needed
183+
ctx.dis.readUnsigned(); // Read and store SRID if needed
186184
}
187185

188186
// Get the base geometry type
@@ -211,7 +209,7 @@ std::unique_ptr<GeoShape> WkbParse::readGeometry(WkbParseContext* ctx) {
211209
}
212210
}
213211

214-
std::unique_ptr<GeoPoint> WkbParse::readPoint(WkbParseContext* ctx) {
212+
std::unique_ptr<GeoPoint> WkbParse::readPoint(WkbParseContext& ctx) {
215213
GeoCoordinateList coords = WkbParse::readCoordinateList(1, ctx);
216214
if (coords.list.empty()) {
217215
return nullptr;
@@ -225,8 +223,8 @@ std::unique_ptr<GeoPoint> WkbParse::readPoint(WkbParseContext* ctx) {
225223
return point;
226224
}
227225

228-
std::unique_ptr<GeoLine> WkbParse::readLine(WkbParseContext* ctx) {
229-
uint32_t size = ctx->dis.readUnsigned();
226+
std::unique_ptr<GeoLine> WkbParse::readLine(WkbParseContext& ctx) {
227+
uint32_t size = ctx.dis.readUnsigned();
230228
if (minMemSize(wkbLine, size, ctx) != GEO_PARSE_OK) {
231229
return nullptr;
232230
}
@@ -244,15 +242,15 @@ std::unique_ptr<GeoLine> WkbParse::readLine(WkbParseContext* ctx) {
244242
return line;
245243
}
246244

247-
std::unique_ptr<GeoPolygon> WkbParse::readPolygon(WkbParseContext* ctx) {
248-
uint32_t num_loops = ctx->dis.readUnsigned();
245+
std::unique_ptr<GeoPolygon> WkbParse::readPolygon(WkbParseContext& ctx) {
246+
uint32_t num_loops = ctx.dis.readUnsigned();
249247
if (minMemSize(wkbPolygon, num_loops, ctx) != GEO_PARSE_OK) {
250248
return nullptr;
251249
}
252250

253251
GeoCoordinateListList coordss;
254252
for (uint32_t i = 0; i < num_loops; ++i) {
255-
uint32_t size = ctx->dis.readUnsigned();
253+
uint32_t size = ctx.dis.readUnsigned();
256254
if (size < 3) { // A polygon loop must have at least 3 points
257255
return nullptr;
258256
}
@@ -262,7 +260,7 @@ std::unique_ptr<GeoPolygon> WkbParse::readPolygon(WkbParseContext* ctx) {
262260
if (coords->list.empty()) {
263261
return nullptr;
264262
}
265-
coordss.add(coords.release());
263+
coordss.add(std::move(coords));
266264
}
267265

268266
std::unique_ptr<GeoPolygon> polygon = GeoPolygon::create_unique();
@@ -273,22 +271,22 @@ std::unique_ptr<GeoPolygon> WkbParse::readPolygon(WkbParseContext* ctx) {
273271
return polygon;
274272
}
275273

276-
GeoCoordinateList WkbParse::readCoordinateList(unsigned size, WkbParseContext* ctx) {
274+
GeoCoordinateList WkbParse::readCoordinateList(unsigned size, WkbParseContext& ctx) {
277275
GeoCoordinateList coords;
278276
for (uint32_t i = 0; i < size; i++) {
279277
if (!readCoordinate(ctx)) {
280278
return GeoCoordinateList();
281279
}
282280
unsigned int j = 0;
283281
GeoCoordinate coord;
284-
coord.x = ctx->ordValues[j++];
285-
coord.y = ctx->ordValues[j++];
282+
coord.x = ctx.ordValues[j++];
283+
coord.y = ctx.ordValues[j++];
286284
coords.add(coord);
287285
}
288286
return coords;
289287
}
290288

291-
GeoParseStatus WkbParse::minMemSize(int wkbType, uint64_t size, WkbParseContext* ctx) {
289+
GeoParseStatus WkbParse::minMemSize(int wkbType, uint64_t size, WkbParseContext& ctx) {
292290
uint64_t minSize = 0;
293291
constexpr uint64_t minCoordSize = 2 * sizeof(double);
294292
//constexpr uint64_t minPtSize = (1+4) + minCoordSize;
@@ -305,14 +303,14 @@ GeoParseStatus WkbParse::minMemSize(int wkbType, uint64_t size, WkbParseContext*
305303
minSize = size * minLoopSize;
306304
break;
307305
}
308-
if (ctx->dis.size() < minSize) {
306+
if (ctx.dis.size() < minSize) {
309307
return GEO_PARSE_WKB_SYNTAX_ERROR;
310308
}
311309
return GEO_PARSE_OK;
312310
}
313-
bool WkbParse::readCoordinate(WkbParseContext* ctx) {
314-
for (std::size_t i = 0; i < ctx->inputDimension; ++i) {
315-
ctx->ordValues[i] = ctx->dis.readDouble();
311+
bool WkbParse::readCoordinate(WkbParseContext& ctx) {
312+
for (std::size_t i = 0; i < ctx.inputDimension; ++i) {
313+
ctx.ordValues[i] = ctx.dis.readDouble();
316314
}
317315

318316
return true;

‎be/src/geo/wkb_parse.h‎

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -46,26 +46,26 @@ constexpr uint32_t WKB_TYPE_MASK = 0xFF;
4646

4747
class WkbParse {
4848
public:
49-
static GeoParseStatus parse_wkb(std::istream& is, GeoShape** shape);
49+
static GeoParseStatus parse_wkb(std::istream& is, std::unique_ptr<GeoShape>& shape);
5050

51-
static WkbParseContext* read_hex(std::istream& is, WkbParseContext* ctx);
51+
private:
52+
static void read_hex(std::istream& is, WkbParseContext& ctx);
5253

53-
static WkbParseContext* read(std::istream& is, WkbParseContext* ctx);
54+
static void read(std::istream& is, WkbParseContext& ctx);
5455

55-
static std::unique_ptr<GeoShape> readGeometry(WkbParseContext* ctx);
56+
static std::unique_ptr<GeoShape> readGeometry(WkbParseContext& ctx);
5657

57-
private:
58-
static std::unique_ptr<GeoPoint> readPoint(WkbParseContext* ctx);
58+
static std::unique_ptr<GeoPoint> readPoint(WkbParseContext& ctx);
5959

60-
static std::unique_ptr<GeoLine> readLine(WkbParseContext* ctx);
60+
static std::unique_ptr<GeoLine> readLine(WkbParseContext& ctx);
6161

62-
static std::unique_ptr<GeoPolygon> readPolygon(WkbParseContext* ctx);
62+
static std::unique_ptr<GeoPolygon> readPolygon(WkbParseContext& ctx);
6363

64-
static GeoCoordinateList readCoordinateList(unsigned size, WkbParseContext* ctx);
64+
static GeoCoordinateList readCoordinateList(unsigned size, WkbParseContext& ctx);
6565

66-
static GeoParseStatus minMemSize(int wkbType, uint64_t size, WkbParseContext* ctx);
66+
static GeoParseStatus minMemSize(int wkbType, uint64_t size, WkbParseContext& ctx);
6767

68-
static bool readCoordinate(WkbParseContext* ctx);
68+
static bool readCoordinate(WkbParseContext& ctx);
6969
};
7070

7171
} // namespace doris

‎be/src/geo/wkb_parse_ctx.h‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -17,12 +17,12 @@
1717

1818
#pragma once
1919

20-
#include "ByteOrderDataInStream.h"
21-
#include "array"
20+
#include <array>
21+
#include <memory>
2222

23-
namespace doris {
24-
class GeoShape;
25-
}
23+
#include "ByteOrderDataInStream.h"
24+
#include "geo/geo_common.h"
25+
#include "geo/geo_types.h"
2626

2727
struct WkbParseContext {
2828
unsigned int inputDimension = 2;
@@ -33,6 +33,6 @@ struct WkbParseContext {
3333

3434
int srid;
3535

36-
doris::GeoShape* shape = nullptr;
36+
std::unique_ptr<doris::GeoShape> shape = nullptr;
3737
doris::GeoParseStatus parse_status = doris::GEO_PARSE_OK;
3838
};

0 commit comments

Comments
 (0)