Skip to content

Commit fc7209a

Browse files
authored
Merge pull request #320 from wudidapaopao/yuxiaozhe-dev
Fix the potential memory leak issue caused by query result memory not being properly released
2 parents c12bda2 + 0f29e2e commit fc7209a

File tree

3 files changed

+13
-6
lines changed

3 files changed

+13
-6
lines changed

src/Client/ClientBase.cpp

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -523,7 +523,14 @@ try
523523
else
524524
{
525525
// The underlying vector<char> will be freed by `free_result` from Python side.
526-
query_result_memory = new std::vector<char>(4096);
526+
// When the input query contains multiple statements, such as "SELECT 1; SELECT 2". When executing "SELECT 2",
527+
// the query_result_memory corresponding to SELECT 1 has not been "stolen" by function stealQueryOutputVector,
528+
// so it remains non-empty.
529+
if (query_result_memory)
530+
query_result_memory->resize(4096);
531+
else
532+
query_result_memory = new std::vector<char>(4096);
533+
527534
query_result_buf = std::make_shared<WriteBufferFromVector<std::vector<char>>>(*query_result_memory);
528535

529536
out_buf = query_result_buf.get();

src/Client/ClientBase.h

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -107,13 +107,13 @@ class ClientBase
107107
return query_result_memory;
108108
}
109109

110-
/// Steals and returns the query output vector, replacing it with a new one
110+
/// Steals and returns the query output vector.
111+
/// Afterward, the content of query_result_memory is released by the Python side.
111112
std::vector<char> * stealQueryOutputVector()
112113
{
113114
auto * result = query_result_memory;
114-
query_result_memory = new std::vector<char>(4096);
115-
// WriteBufferFromVector takes a reference to the vector but doesn't own it
116-
query_result_buf = std::make_shared<WriteBufferFromVector<std::vector<char>>>(*query_result_memory);
115+
query_result_memory = nullptr;
116+
query_result_buf.reset();
117117
return result;
118118
}
119119

src/Core/ServerSettings.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -126,7 +126,7 @@ namespace DB
126126
M(UInt64, background_fetches_pool_size, 1, "The maximum number of threads that will be used for fetching data parts from another replica for *MergeTree-engine tables in a background.", 0) \
127127
M(UInt64, background_common_pool_size, 1, "The maximum number of threads that will be used for performing a variety of operations (mostly garbage collection) for *MergeTree-engine tables in a background.", 0) \
128128
M(UInt64, background_buffer_flush_schedule_pool_size, 2, "The maximum number of threads that will be used for performing flush operations for Buffer-engine tables in a background.", 0) \
129-
M(UInt64, background_schedule_pool_size, 1, "The maximum number of threads that will be used for constantly executing some lightweight periodic operations.", 0) \
129+
M(UInt64, background_schedule_pool_size, 2, "The maximum number of threads that will be used for constantly executing some lightweight periodic operations.", 0) \
130130
M(UInt64, background_message_broker_schedule_pool_size, 0, "The maximum number of threads that will be used for executing background operations for message streaming.", 0) \
131131
M(UInt64, background_distributed_schedule_pool_size, 0, "The maximum number of threads that will be used for executing distributed sends.", 0) \
132132
M(UInt64, tables_loader_foreground_pool_size, 0, "The maximum number of threads that will be used for foreground (that is being waited for by a query) loading of tables. Also used for synchronous loading of tables before the server start. Zero means use all CPUs.", 0) \

0 commit comments

Comments
 (0)