Skip to content

Commit

Permalink
fix memory leak in RowTuple::set_schema (#457)
Browse files Browse the repository at this point in the history
### What problem were solved in this pull request?

**Problem:**

When I attempted to join multiple tables, `libsanitizer` detected a
memory leak. The error log is as follows:

```
==23682==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 768 byte(s) in 12 object(s) allocated from:
    #0 0x75f738946a2a in operator new(unsigned long) /usr/src/debug/gcc/gcc/libsanitizer/asan/asan_new_delete.cpp:95
    #1 0x56870207d56f in RowTuple::set_schema(Table const*, std::vector<FieldMeta, std::allocator<FieldMeta> > const*) /home/pepefloyd/git/miniob/src/observer/sql/expr/tuple.h:184
    #2 0x56870208f737 in TableScanPhysicalOperator::open(Trx*) /home/pepefloyd/git/miniob/src/observer/sql/operator/table_scan_physical_operator.cpp:25
    #3 0x568702081888 in NestedLoopJoinPhysicalOperator::right_next() /home/pepefloyd/git/miniob/src/observer/sql/operator/join_physical_operator.cpp:113
    #4 0x568702083b42 in PredicatePhysicalOperator::next() /home/pepefloyd/git/miniob/src/observer/sql/operator/predicate_physical_operator.cpp:41
    #5 0x56870206f411 in HashGroupByPhysicalOperator::open(Trx*) /home/pepefloyd/git/miniob/src/observer/sql/operator/hash_group_by_physical_operator.cpp:44
    #6 0x568702085740 in ProjectPhysicalOperator::open(Trx*) /home/pepefloyd/git/miniob/src/observer/sql/operator/project_physical_operator.cpp:34
    #7 0x568702038e2c in PlainCommunicator::write_result_internal(SessionEvent*, bool&) /home/pepefloyd/git/miniob/src/observer/net/plain_communicator.cpp:193
    #8 0x56870203a7cc in PlainCommunicator::write_result(SessionEvent*, bool&) /home/pepefloyd/git/miniob/src/observer/net/plain_communicator.cpp:162
    #9 0x568701ee5e4b in SqlTaskHandler::handle_event(Communicator*) /home/pepefloyd/git/miniob/src/observer/net/sql_task_handler.cpp:45
    #10 0x56870202f0d7 in Worker::operator()() /home/pepefloyd/git/miniob/src/observer/net/one_thread_per_connection_thread_handler.cpp:96
    #11 0x75f7384f78b3 in execute_native_thread_routine /usr/src/debug/gcc/gcc/libstdc++-v3/src/c++11/thread.cc:104

SUMMARY: AddressSanitizer: 768 byte(s) leaked in 12 allocation(s).
```

### What is changed and how it works?

To fix the memory leak, I identified that in the function
`Tuple::set_schema`:

```cpp
void set_schema(const Table *table, const std::vector<FieldMeta> *fields)
{
  table_ = table;
  // fix: During a join, the right table's open is called multiple times,
  // which invokes set_schema repeatedly. This causes Tuple to store
  // unnecessary fields and values, so it is necessary to clear the existing ones first.
  this->speces_.clear();
  this->speces_.reserve(fields->size());
  for (const FieldMeta &field : *fields) {
    speces_.push_back(new FieldExpr(table, &field));
  }
}
```

The call to `this->speces_.clear()` does not destruct the existing
`FieldExpr*` objects. As a result, the memory was not released properly.
The fix I applied is to manually delete the `FieldExpr*` before clearing
the vector, as shown below:

```cpp
for (FieldExpr *spec : speces_) {
  delete spec;
}
```

This ensures that memory allocated for `FieldExpr` is properly released,
thus fixing the memory leak.
  • Loading branch information
sisabyss authored Oct 11, 2024
1 parent 09ee2f4 commit 8d1188d
Showing 1 changed file with 3 additions and 0 deletions.
3 changes: 3 additions & 0 deletions src/observer/sql/expr/tuple.h
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,9 @@ class RowTuple : public Tuple
table_ = table;
// fix:join当中会多次调用右表的open,open当中会调用set_scheme,从而导致tuple当中会存储
// 很多无意义的field和value,因此需要先clear掉
for (FieldExpr *spec : speces_) {
delete spec;
}
this->speces_.clear();
this->speces_.reserve(fields->size());
for (const FieldMeta &field : *fields) {
Expand Down

0 comments on commit 8d1188d

Please sign in to comment.