Skip to content

Commit

Permalink
avoid leaking thread names at thread exit
Browse files Browse the repository at this point in the history
Summary:
gcc 5 on travis had an asan reported error:

```
==11639==ERROR: LeakSanitizer: detected memory leaks
Direct leak of 13 byte(s) in 1 object(s) allocated from:
    #0 0x2b0094f9657a in malloc (/usr/lib/x86_64-linux-gnu/libasan.so.2+0x9757a)
    #1 0x2b009691ffe7 in vasprintf (/lib/x86_64-linux-gnu/libc.so.6+0x71fe7)
SUMMARY  AddressSanitizer: 13 byte(s) leaked in 1 allocation(s).
FAIL: tests/future.t
1..17
ok 1 - some/path expected=0 actual=0
ok 2 - buck-out/gen/foo expected=1 actual=1
ok 3 - .hg/wlock expected=0 actual=0
ok 4 - .hg/store/foo expected=1 actual=1
ok 5 - buck-out expected=1 actual=1
ok 6 - foo/buck-out expected=1 actual=1
ok 7 - foo/hello expected=0 actual=0
ok 8 - baz/hello expected=0 actual=0
ok 9 - .hg expected=0 actual=0
ok 10 - buil expected=0 actual=0
ok 11 - build expected=1 actual=1
ok 12 - build/lower expected=1 actual=1
ok 13 - builda expected=0 actual=0
ok 14 - build/bar expected=1 actual=1
ok 15 - buildfile expected=0 actual=0
ok 16 - build/lower/baz expected=1 actual=1
ok 17 - builda/hello expected=0 actual=0
```

looking at the code, we now only use vasprintf in the TAP test harness and
for thread names.

We only register a dtor for the thread local storage on darwin, which doesn't
support C++11 thread_local storage duration.

Since windows doesn't have pthread_setspecific we do prefer a solution that
works with the C++11 thread_local storage duration, so this diff changes
the type of the thread local to std::string on everything other than darwin,
so that its dtor will run at the appropriate time.

Differential Revision: D5101485

fbshipit-source-id: d7fd37bdcb8c3928b53b27dd617f3aa41cc919fa
  • Loading branch information
wez authored and facebook-github-bot committed May 20, 2017
1 parent 56f8711 commit 05ee68f
Showing 1 changed file with 14 additions and 5 deletions.
19 changes: 14 additions & 5 deletions log.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,13 @@
#include <pthread.h>
#endif
#include "Logging.h"
#include "watchman_scopeguard.h"

int log_level = W_LOG_ERR;
#ifdef __APPLE__
static pthread_key_t thread_name_key;
#else
static thread_local char *thread_name = nullptr;
static thread_local std::string thread_name_str;
#endif
static constexpr size_t kMaxFrames = 64;

Expand Down Expand Up @@ -122,6 +123,8 @@ char* Log::currentTimeString(char* buf, size_t bufsize) {
const char* Log::getThreadName() {
#ifdef __APPLE__
auto thread_name = (char*)pthread_getspecific(thread_name_key);
#else
auto thread_name = thread_name_str.c_str();
#endif

if (thread_name) {
Expand Down Expand Up @@ -336,19 +339,25 @@ const char *w_set_thread_name(const char *fmt, ...) {
va_list ap;
#ifdef __APPLE__
auto thread_name = (char*)pthread_getspecific(thread_name_key);
#endif

free(thread_name);
#else
char* thread_name = nullptr;
SCOPE_EXIT {
free(thread_name);
};
#endif

va_start(ap, fmt);
ignore_result(vasprintf(&thread_name, fmt, ap));
va_end(ap);

#ifdef __APPLE__
pthread_setspecific(thread_name_key, thread_name);
#endif

return thread_name;
#else
thread_name_str = thread_name;
return thread_name_str.c_str();
#endif
}

void w_log(int level, WATCHMAN_FMT_STRING(const char *fmt), ...)
Expand Down

0 comments on commit 05ee68f

Please sign in to comment.