apache / apache/brpc

bthread: bthread_setspecific may cause memory leak

Open
#1,449 5 comments 0 reactions 0 assignees View on GitHub
bug
Dominant language
C++
Stars
17.6k
Forks
4.1k
Avg merge
2d 12h
Merged PRs (30d)
69

Description

**Describe the bug (描述bug)**
In the implementation of `bthread_setspecific`, if the current bthread has no KeyTable, a new KeyTable will be created. When the bthread ends, the created KeyTable will not be destroyed but returned to a KeyTablePool, which causes a memory leak. If we call `bthread_getspecific` before `bthread_setspecific`, the memory leak will not happen because `bthread_getspecific` may borrow a KeyTable from KeyTablePool and make `bthread_setspecific` reuse it.

This memory leak can be reproduced except these situations:
1. using `brpc::thread_local_data()` before `bthread_setspecific`. `brpc::thread_local_data()` calls `bthread_getspecific` which may borrow a KeyTable from KeyTablePool, after that, all `bthread_setspecific` will use this KeyTabe instead of creating new one.
2. using LOG() before `bthread_setspecific`, the reason is the same as above, the implementation of LOG() in brpc creates a LogStream which uses `bthread_local`, and it calls `bthread_getspecific` at first.
3. calling `bthread_getspecific` before `bthread_setspecific`, also the same as above.

**To Reproduce (复现方法)**
we can simply modify the `echo_c++` in `example` to reproduce this behavior, the key code just like this.

```
brpc::ClosureGuard done_guard(done);

brpc::Controller* cntl =
static_cast(cntl_base);

if (bthread_setspecific(key1, static_cast(100)) != 0) {
//
}

```

and we can add log in `bthread_setspecific` to see if a KeyTable created or not, the result is yes.

**Expected behavior (期望行为)**
users can just call `bthread_setspecific`, instead of calling `bthread_getspecific` at first.

**Versions (各种版本)**
OS: Ubuntu 18.04.5 LTS
Compiler: g++ (Ubuntu 7.5.0-3ubuntu1~18.04) 7.5.0
brpc: master branch, commit id: 06505824def0e7bf23e548ff102e040363d31e38
protobuf: 3.0.0-9.1ubuntu1

**Additional context/screenshots (更多上下文/截图)**
The `bthread_setspecific` will create a new KeyTable and not borrow one from KeyTablePool
![image](https://user-images.githubusercontent.com/16054841/123507502-5ee15500-d69c-11eb-88a5-9994c6be6adc.png)
The KeyTable will return to KeyTablePool when bthread ends.
![image](https://user-images.githubusercontent.com/16054841/123507550-a1a32d00-d69c-11eb-82ca-60bc4cfb1474.png)
The `bthread_getspecific` may borrow a KeyTable from KeyTablePool.
![image](https://user-images.githubusercontent.com/16054841/123507536-8801e580-d69c-11eb-9fef-dc5224ee96a0.png)

Contributor guide

Open the contributing guide

Research direction

Start by tracing the bthread_setspecific, bthread_getspecific, and KeyTablePool entry points described in the issue, then reproduce the behavior with the modified echo_c++ example. Compare the KeyTable lifecycle when bthread_setspecific is called directly versus after bthread_getspecific. Done means the direct call no longer leaves the reported memory leak while preserving the expected specific-value behavior.

Written by the indexing model from the issue text.

Assessment

Tech stack
cpp
Domain
operating-systems
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
42/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.