Skip to content

Commit 01f640a

Browse files
ymasmoudifacebook-github-bot
authored andcommitted
Fix mem leak in GRPCReceiver (magma#1833)
Summary: Pull Request resolved: magma#1833 Under heavy load, Sessiond crashes multiple times during memory allocation. ``` (gdb) frame 4 #4 0x00007f857cd2a62f in _int_malloc (av=av@entry=0x7f857d04ab00 <main_arena>, bytes=bytes@entry=40) at malloc.c:3765 3765 malloc.c: No such file or directory. (gdb) info locals p = 0x55a9f11e7cc0 iters = <optimized out> nb = 48 idx = 4 bin = <optimized out> victim = 0x55a9f11e7cc0 ... remainder_size = 528 block = <optimized out> ... errstr = 0x0 func = "_int_malloc" (gdb) p victim $1 = {prev_size = 13249033401518231684, size = 577, fd = 0x7f857d04ad88 <main_arena+648>, bk = 0x55a9f1184180, fd_nextsize = 0x0, bk_nextsize = 0x55a9f057dae1 <std::_Function_base::Base_manager<magma::AsyncPipelinedClient::deactivate_flows_for_rules(const string&, const std::vector<std::cxx11::basic_string<char> >&, const std::vectormagma::lte::PolicyRule&, magma::lte::RequestOriginType_OriginType)::<lambda(grpc::Status, magma::lte::DeactivateFlowsResult)> >::M_manager(std::Any_data &, const std::Any_data &, std::Manager_operation)>} ``` Crash address hints that the issue is related to aync calls to pipelined/directoryd/AAAclient. Looking at the code of GRPCReceiver.h, it seems that we could hit a memory corruption if the AsyncLocalResponse object is deleted in handle_response before moving the reader to the response_reader_ attribute. ``` 78 void set_response_reader( 79 std::unique_ptr<grpc::ClientAsyncResponseReader<ResponseType>> reader) { 80 reader->Finish(&response, &status, this); 81 response_reader_ = std::move(reader); 82 } ... 121 void handle_response() { 122 this->callback(this->status, this->response); 123 delete this; 124 } ``` This can be confirmed by printing the address of this and this->response_reader_ just before the delete. the latest would be NULL just before the sigfault. In addition, the destructor is set to virtual as the class has a virtual method to force calling the correct destructor for sub-classes. Reviewed By: themarwhal Differential Revision: D22066268 fbshipit-source-id: c45c32a031e26bbd86c80f1d0c2286f4015abff2
1 parent ac98b47 commit 01f640a

File tree

1 file changed

+2
-1
lines changed

1 file changed

+2
-1
lines changed

orc8r/gateway/c/common/async_grpc/GRPCReceiver.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,7 @@ class AsyncGRPCResponse : public AsyncResponse {
6767
context_.set_deadline(
6868
std::chrono::system_clock::now() + std::chrono::seconds(timeout_sec));
6969
}
70+
virtual ~AsyncGRPCResponse() = default;
7071

7172
virtual void handle_response() {}
7273

@@ -76,8 +77,8 @@ class AsyncGRPCResponse : public AsyncResponse {
7677
*/
7778
void set_response_reader(
7879
std::unique_ptr<grpc::ClientAsyncResponseReader<ResponseType>> reader) {
79-
reader->Finish(&response_, &status_, this);
8080
response_reader_ = std::move(reader);
81+
response_reader_->Finish(&response_, &status_, this);
8182
}
8283

8384
/**

0 commit comments

Comments
 (0)