Skip to content

Commit 8e56b5f

Browse files
committed
ci: fix memory leak and enable sanitizer ci
1 parent 3cf5963 commit 8e56b5f

File tree

2 files changed

+54
-10
lines changed

2 files changed

+54
-10
lines changed

.github/workflows/sanitizer_test.yml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -50,9 +50,9 @@ jobs:
5050
- name: Run Tests
5151
working-directory: build
5252
env:
53-
ASAN_OPTIONS: log_path=out.log:detect_leaks=1:symbolize=1:strict_string_checks=1:halt_on_error=0:detect_container_overflow=0
53+
ASAN_OPTIONS: log_path=out.log:detect_leaks=1:symbolize=1:strict_string_checks=1:halt_on_error=1:detect_container_overflow=0
5454
LSAN_OPTIONS: suppressions=${{ github.workspace }}/.github/lsan-suppressions.txt
55-
UBSAN_OPTIONS: log_path=out.log:halt_on_error=0:print_stacktrace=1:suppressions=${{ github.workspace }}/.github/ubsan-suppressions.txt
55+
UBSAN_OPTIONS: log_path=out.log:halt_on_error=1:print_stacktrace=1:suppressions=${{ github.workspace }}/.github/ubsan-suppressions.txt
5656
run: |
5757
ctest --output-on-failure
5858
- name: Save the test output

test/arrow_test.cc

Lines changed: 52 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,42 @@
3535

3636
namespace iceberg {
3737

38+
namespace {
39+
40+
// A RAII guard for ArrowSchema.
41+
struct ArrowSchemaGuard {
42+
ArrowSchema* schema;
43+
44+
~ArrowSchemaGuard() {
45+
if (schema != nullptr) {
46+
ReleaseRecursively(schema);
47+
}
48+
}
49+
50+
private:
51+
void ReleaseRecursively(ArrowSchema* schema_ptr) {
52+
if (schema_ptr == nullptr) {
53+
return;
54+
}
55+
56+
// First, recursively release all children
57+
if (schema_ptr->children != nullptr && schema_ptr->n_children > 0) {
58+
for (int64_t i = 0; i < schema_ptr->n_children; ++i) {
59+
if (schema_ptr->children[i] != nullptr) {
60+
ReleaseRecursively(schema_ptr->children[i]);
61+
}
62+
}
63+
}
64+
65+
// Then release this schema if it has a release callback
66+
if (schema_ptr->release != nullptr) {
67+
schema_ptr->release(schema_ptr);
68+
}
69+
}
70+
};
71+
72+
} // namespace
73+
3874
TEST(ArrowCDataTest, CheckArrowSchemaAndArrayByNanoarrow) {
3975
auto [schema, array] = internal::CreateExampleArrowSchemaAndArrayByNanoarrow();
4076

@@ -86,7 +122,8 @@ TEST_P(ToArrowSchemaTest, PrimitiveType) {
86122
: SchemaField::MakeRequired(kFieldId, std::string(kFieldName),
87123
param.iceberg_type)},
88124
/*schema_id=*/0);
89-
ArrowSchema arrow_schema;
125+
ArrowSchema arrow_schema{};
126+
ArrowSchemaGuard guard{&arrow_schema};
90127
ASSERT_THAT(ToArrowSchema(schema, &arrow_schema), IsOk());
91128

92129
auto imported_schema = ::arrow::ImportSchema(&arrow_schema).ValueOrDie();
@@ -170,7 +207,8 @@ TEST(ToArrowSchemaTest, StructType) {
170207
struct_type)},
171208
/*schema_id=*/0);
172209

173-
ArrowSchema arrow_schema;
210+
ArrowSchema arrow_schema{};
211+
ArrowSchemaGuard guard{&arrow_schema};
174212
ASSERT_THAT(ToArrowSchema(schema, &arrow_schema), IsOk());
175213

176214
auto imported_schema = ::arrow::ImportSchema(&arrow_schema).ValueOrDie();
@@ -202,7 +240,8 @@ TEST(ToArrowSchemaTest, ListType) {
202240
{SchemaField::MakeRequired(kListFieldId, std::string(kListFieldName), list_type)},
203241
/*schema_id=*/0);
204242

205-
ArrowSchema arrow_schema;
243+
ArrowSchema arrow_schema{};
244+
ArrowSchemaGuard guard{&arrow_schema};
206245
ASSERT_THAT(ToArrowSchema(schema, &arrow_schema), IsOk());
207246

208247
auto imported_schema = ::arrow::ImportSchema(&arrow_schema).ValueOrDie();
@@ -237,7 +276,8 @@ TEST(ToArrowSchemaTest, MapType) {
237276
{SchemaField::MakeRequired(kFieldId, std::string(kMapFieldName), map_type)},
238277
/*schema_id=*/0);
239278

240-
ArrowSchema arrow_schema;
279+
ArrowSchema arrow_schema{};
280+
ArrowSchemaGuard guard{&arrow_schema};
241281
ASSERT_THAT(ToArrowSchema(schema, &arrow_schema), IsOk());
242282

243283
auto imported_schema = ::arrow::ImportSchema(&arrow_schema).ValueOrDie();
@@ -278,7 +318,8 @@ TEST_P(FromArrowSchemaTest, PrimitiveType) {
278318
{std::string(kFieldIdKey), std::to_string(kFieldId)}});
279319
auto arrow_schema = ::arrow::schema({::arrow::field(
280320
std::string(kFieldName), param.arrow_type, param.optional, std::move(metadata))});
281-
ArrowSchema exported_schema;
321+
ArrowSchema exported_schema{};
322+
ArrowSchemaGuard guard{&exported_schema};
282323
ASSERT_TRUE(::arrow::ExportSchema(*arrow_schema, &exported_schema).ok());
283324

284325
auto type_result = FromArrowSchema(exported_schema, /*schema_id=*/1);
@@ -353,7 +394,8 @@ TEST(FromArrowSchemaTest, StructType) {
353394
::arrow::key_value_metadata(std::unordered_map<std::string, std::string>{
354395
{std::string(kFieldIdKey), std::to_string(kStructFieldId)}}));
355396
auto arrow_schema = ::arrow::schema({struct_field});
356-
ArrowSchema exported_schema;
397+
ArrowSchema exported_schema{};
398+
ArrowSchemaGuard guard{&exported_schema};
357399
ASSERT_TRUE(::arrow::ExportSchema(*arrow_schema, &exported_schema).ok());
358400

359401
auto schema_result = FromArrowSchema(exported_schema, /*schema_id=*/0);
@@ -403,7 +445,8 @@ TEST(FromArrowSchemaTest, ListType) {
403445
{std::string(kFieldIdKey), std::to_string(kListFieldId)}}));
404446
auto arrow_schema = ::arrow::schema({list_field});
405447

406-
ArrowSchema exported_schema;
448+
ArrowSchema exported_schema{};
449+
ArrowSchemaGuard guard{&exported_schema};
407450
ASSERT_TRUE(::arrow::ExportSchema(*arrow_schema, &exported_schema).ok());
408451

409452
auto schema_result = FromArrowSchema(exported_schema, /*schema_id=*/0);
@@ -453,7 +496,8 @@ TEST(FromArrowSchemaTest, MapType) {
453496
{std::string(kFieldIdKey), std::to_string(kFieldId)}}));
454497
auto arrow_schema = ::arrow::schema({map_field});
455498

456-
ArrowSchema exported_schema;
499+
ArrowSchema exported_schema{};
500+
ArrowSchemaGuard guard{&exported_schema};
457501
ASSERT_TRUE(::arrow::ExportSchema(*arrow_schema, &exported_schema).ok());
458502

459503
auto schema_result = FromArrowSchema(exported_schema, /*schema_id=*/0);

0 commit comments

Comments
 (0)