diff --git a/include/jemalloc/internal/prof_data.h b/include/jemalloc/internal/prof_data.h index 4c8e22c7..c4286b51 100644 --- a/include/jemalloc/internal/prof_data.h +++ b/include/jemalloc/internal/prof_data.h @@ -18,7 +18,6 @@ bool prof_bt_keycomp(const void *k1, const void *k2); bool prof_data_init(tsd_t *tsd); prof_tctx_t *prof_lookup(tsd_t *tsd, prof_bt_t *bt); -char *prof_thread_name_alloc(tsd_t *tsd, const char *thread_name); int prof_thread_name_set_impl(tsd_t *tsd, const char *thread_name); void prof_unbias_map_init(); void prof_dump_impl(tsd_t *tsd, write_cb_t *prof_dump_write, void *cbopaque, diff --git a/include/jemalloc/internal/prof_inlines.h b/include/jemalloc/internal/prof_inlines.h index ab3e01f6..b74b115c 100644 --- a/include/jemalloc/internal/prof_inlines.h +++ b/include/jemalloc/internal/prof_inlines.h @@ -38,6 +38,22 @@ prof_gdump_get_unlocked(void) { return prof_gdump_val; } +JEMALLOC_ALWAYS_INLINE void +prof_thread_name_assert(prof_tdata_t *tdata) { + if (!config_debug) { + return; + } + prof_active_assert(); + + bool terminated = false; + for (unsigned i = 0; i < PROF_THREAD_NAME_MAX_LEN; i++) { + if (tdata->thread_name[i] == '\0') { + terminated = true; + } + } + assert(terminated); +} + JEMALLOC_ALWAYS_INLINE prof_tdata_t * prof_tdata_get(tsd_t *tsd, bool create) { prof_tdata_t *tdata; @@ -59,6 +75,10 @@ prof_tdata_get(tsd_t *tsd, bool create) { assert(tdata == NULL || tdata->attached); } + if (tdata != NULL) { + prof_thread_name_assert(tdata); + } + return tdata; } @@ -255,4 +275,18 @@ prof_free(tsd_t *tsd, const void *ptr, size_t usize, } } +JEMALLOC_ALWAYS_INLINE bool +prof_thread_name_empty(prof_tdata_t *tdata) { + prof_active_assert(); + + return (tdata->thread_name[0] == '\0'); +} + +JEMALLOC_ALWAYS_INLINE void +prof_thread_name_clear(prof_tdata_t *tdata) { + prof_active_assert(); + + tdata->thread_name[0] = '\0'; +} + #endif /* JEMALLOC_INTERNAL_PROF_INLINES_H */ diff --git a/include/jemalloc/internal/prof_structs.h b/include/jemalloc/internal/prof_structs.h index 9331fba4..da3cf8d5 100644 --- a/include/jemalloc/internal/prof_structs.h +++ b/include/jemalloc/internal/prof_structs.h @@ -156,9 +156,6 @@ struct prof_tdata_s { */ uint64_t thr_discrim; - /* Included in heap profile dumps if non-NULL. */ - char *thread_name; - bool attached; bool expired; @@ -179,6 +176,9 @@ struct prof_tdata_s { */ ckh_t bt2tctx; + /* Included in heap profile dumps if has content. */ + char thread_name[PROF_THREAD_NAME_MAX_LEN]; + /* State used to avoid dumping while operating on prof internals. */ bool enq; bool enq_idump; diff --git a/include/jemalloc/internal/prof_types.h b/include/jemalloc/internal/prof_types.h index 87cbb4ab..104f7e61 100644 --- a/include/jemalloc/internal/prof_types.h +++ b/include/jemalloc/internal/prof_types.h @@ -77,4 +77,7 @@ typedef struct prof_recent_s prof_recent_t; /* Default number of recent allocations to record. */ #define PROF_RECENT_ALLOC_MAX_DEFAULT 0 +/* Thread name storage size limit. */ +#define PROF_THREAD_NAME_MAX_LEN 16 + #endif /* JEMALLOC_INTERNAL_PROF_TYPES_H */ diff --git a/src/ctl.c b/src/ctl.c index eafbdc61..cfd4ac6e 100644 --- a/src/ctl.c +++ b/src/ctl.c @@ -2384,13 +2384,13 @@ thread_prof_name_ctl(tsd_t *tsd, const size_t *mib, READ_XOR_WRITE(); if (newp != NULL) { - if (newlen != sizeof(const char *)) { + const char *newval = *(const char **)newp; + if (newlen != sizeof(const char *) || newval == NULL) { ret = EINVAL; goto label_return; } - if ((ret = prof_thread_name_set(tsd, *(const char **)newp)) != - 0) { + if ((ret = prof_thread_name_set(tsd, newval)) != 0) { goto label_return; } } else { diff --git a/src/prof.c b/src/prof.c index 91425371..832aa528 100644 --- a/src/prof.c +++ b/src/prof.c @@ -415,11 +415,14 @@ prof_tdata_t * prof_tdata_reinit(tsd_t *tsd, prof_tdata_t *tdata) { uint64_t thr_uid = tdata->thr_uid; uint64_t thr_discrim = tdata->thr_discrim + 1; - char *thread_name = (tdata->thread_name != NULL) ? - prof_thread_name_alloc(tsd, tdata->thread_name) : NULL; bool active = tdata->active; + /* Keep a local copy of the thread name, before detaching. */ + prof_thread_name_assert(tdata); + char thread_name[PROF_THREAD_NAME_MAX_LEN]; + strncpy(thread_name, tdata->thread_name, PROF_THREAD_NAME_MAX_LEN); prof_tdata_detach(tsd, tdata); + return prof_tdata_init_impl(tsd, thr_uid, thr_discrim, thread_name, active); } @@ -464,15 +467,15 @@ prof_active_set(tsdn_t *tsdn, bool active) { const char * prof_thread_name_get(tsd_t *tsd) { + static const char *prof_thread_name_dummy = ""; + assert(tsd_reentrancy_level_get(tsd) == 0); - - prof_tdata_t *tdata; - - tdata = prof_tdata_get(tsd, true); + prof_tdata_t *tdata = prof_tdata_get(tsd, true); if (tdata == NULL) { - return ""; + return prof_thread_name_dummy; } - return (tdata->thread_name != NULL ? tdata->thread_name : ""); + + return tdata->thread_name; } int diff --git a/src/prof_data.c b/src/prof_data.c index 56d3dc88..c33668ee 100644 --- a/src/prof_data.c +++ b/src/prof_data.c @@ -441,64 +441,30 @@ prof_bt_count(void) { return bt_count; } -char * -prof_thread_name_alloc(tsd_t *tsd, const char *thread_name) { - char *ret; - size_t size; - - if (thread_name == NULL) { - return NULL; - } - - size = strlen(thread_name) + 1; - ret = iallocztm(tsd_tsdn(tsd), size, sz_size2index(size), false, NULL, - true, arena_get(TSDN_NULL, 0, true), true); - if (ret == NULL) { - return NULL; - } - - memcpy(ret, thread_name, size); - ret[size - 1] = '\0'; - - return ret; +static void +prof_thread_name_write_tdata(prof_tdata_t *tdata, const char *thread_name) { + strncpy(tdata->thread_name, thread_name, PROF_THREAD_NAME_MAX_LEN); + tdata->thread_name[PROF_THREAD_NAME_MAX_LEN - 1] = '\0'; } int prof_thread_name_set_impl(tsd_t *tsd, const char *thread_name) { assert(tsd_reentrancy_level_get(tsd) == 0); + assert(thread_name != NULL); - prof_tdata_t *tdata; - unsigned i; - char *s; - - tdata = prof_tdata_get(tsd, true); - if (tdata == NULL) { - return EAGAIN; - } - - /* Validate input. */ - if (thread_name == NULL) { - return EFAULT; - } - for (i = 0; thread_name[i] != '\0'; i++) { + for (unsigned i = 0; thread_name[i] != '\0'; i++) { char c = thread_name[i]; if (!isgraph(c) && !isblank(c)) { - return EFAULT; + return EINVAL; } } - s = prof_thread_name_alloc(tsd, thread_name); - if (s == NULL) { - return EAGAIN; + prof_tdata_t *tdata = prof_tdata_get(tsd, true); + if (tdata == NULL) { + return ENOMEM; } - char *old_thread_name = tdata->thread_name; - tdata->thread_name = s; - if (old_thread_name != NULL) { - idalloctm(tsd_tsdn(tsd), old_thread_name, /* tcache */ NULL, - /* alloc_ctx */ NULL, /* is_internal */ true, - /* slow_path */ true); - } + prof_thread_name_write_tdata(tdata, thread_name); return 0; } @@ -949,7 +915,7 @@ prof_tdata_dump_iter(prof_tdata_tree_t *tdatas_ptr, prof_tdata_t *tdata, tdata->thr_uid); prof_dump_print_cnts(arg->prof_dump_write, arg->cbopaque, &tdata->cnt_summed); - if (tdata->thread_name != NULL) { + if (!prof_thread_name_empty(tdata)) { arg->prof_dump_write(arg->cbopaque, " "); arg->prof_dump_write(arg->cbopaque, tdata->thread_name); } @@ -1179,10 +1145,15 @@ prof_tdata_init_impl(tsd_t *tsd, uint64_t thr_uid, uint64_t thr_discrim, tdata->lock = prof_tdata_mutex_choose(thr_uid); tdata->thr_uid = thr_uid; tdata->thr_discrim = thr_discrim; - tdata->thread_name = thread_name; tdata->attached = true; tdata->expired = false; tdata->tctx_uid_next = 0; + if (thread_name == NULL) { + prof_thread_name_clear(tdata); + } else { + prof_thread_name_write_tdata(tdata, thread_name); + } + prof_thread_name_assert(tdata); if (ckh_new(tsd, &tdata->bt2tctx, PROF_CKH_MINITEMS, prof_bt_hash, prof_bt_keycomp)) { @@ -1230,13 +1201,8 @@ prof_tdata_destroy_locked(tsd_t *tsd, prof_tdata_t *tdata, malloc_mutex_assert_not_owner(tsd_tsdn(tsd), tdata->lock); tdata_tree_remove(&tdatas, tdata); - assert(prof_tdata_should_destroy_unlocked(tdata, even_if_attached)); - if (tdata->thread_name != NULL) { - idalloctm(tsd_tsdn(tsd), tdata->thread_name, NULL, NULL, true, - true); - } ckh_delete(tsd, &tdata->bt2tctx); idalloctm(tsd_tsdn(tsd), tdata, NULL, NULL, true, true); } diff --git a/src/prof_log.c b/src/prof_log.c index 0632c3b3..384d5e38 100644 --- a/src/prof_log.c +++ b/src/prof_log.c @@ -243,8 +243,7 @@ prof_try_log(tsd_t *tsd, size_t usize, prof_info_t *prof_info) { iallocztm(tsd_tsdn(tsd), sz, sz_size2index(sz), false, NULL, true, arena_get(TSDN_NULL, 0, true), true); - const char *prod_thr_name = (tctx->tdata->thread_name == NULL)? - "" : tctx->tdata->thread_name; + const char *prod_thr_name = tctx->tdata->thread_name; const char *cons_thr_name = prof_thread_name_get(tsd); prof_bt_t bt; diff --git a/src/prof_recent.c b/src/prof_recent.c index 834a9446..4c3c6296 100644 --- a/src/prof_recent.c +++ b/src/prof_recent.c @@ -495,7 +495,7 @@ prof_recent_alloc_dump_node(emitter_t *emitter, prof_recent_t *node) { &node->alloc_tctx->thr_uid); prof_tdata_t *alloc_tdata = node->alloc_tctx->tdata; assert(alloc_tdata != NULL); - if (alloc_tdata->thread_name != NULL) { + if (!prof_thread_name_empty(alloc_tdata)) { emitter_json_kv(emitter, "alloc_thread_name", emitter_type_string, &alloc_tdata->thread_name); } @@ -511,7 +511,7 @@ prof_recent_alloc_dump_node(emitter_t *emitter, prof_recent_t *node) { emitter_type_uint64, &node->dalloc_tctx->thr_uid); prof_tdata_t *dalloc_tdata = node->dalloc_tctx->tdata; assert(dalloc_tdata != NULL); - if (dalloc_tdata->thread_name != NULL) { + if (!prof_thread_name_empty(dalloc_tdata)) { emitter_json_kv(emitter, "dalloc_thread_name", emitter_type_string, &dalloc_tdata->thread_name); } diff --git a/src/prof_sys.c b/src/prof_sys.c index d2487fd6..3f7196f8 100644 --- a/src/prof_sys.c +++ b/src/prof_sys.c @@ -462,12 +462,17 @@ prof_sys_thread_name_read_t *JET_MUTABLE prof_sys_thread_name_read = void prof_sys_thread_name_fetch(tsd_t *tsd) { -#define THREAD_NAME_MAX_LEN 16 - char buf[THREAD_NAME_MAX_LEN]; - if (!prof_sys_thread_name_read(buf, THREAD_NAME_MAX_LEN)) { - prof_thread_name_set_impl(tsd, buf); + prof_tdata_t *tdata = prof_tdata_get(tsd, true); + if (tdata == NULL) { + return; } -#undef THREAD_NAME_MAX_LEN + + if (prof_sys_thread_name_read(tdata->thread_name, + PROF_THREAD_NAME_MAX_LEN) != 0) { + prof_thread_name_clear(tdata); + } + + tdata->thread_name[PROF_THREAD_NAME_MAX_LEN - 1] = '\0'; } int diff --git a/test/unit/prof_thread_name.c b/test/unit/prof_thread_name.c index 3c4614fc..0fc29f75 100644 --- a/test/unit/prof_thread_name.c +++ b/test/unit/prof_thread_name.c @@ -14,8 +14,6 @@ mallctl_thread_name_get_impl(const char *thread_name_expected, const char *func, expect_str_eq(thread_name_old, thread_name_expected, "%s():%d: Unexpected thread.prof.name value", func, line); } -#define mallctl_thread_name_get(a) \ - mallctl_thread_name_get_impl(a, __func__, __LINE__) static void mallctl_thread_name_set_impl(const char *thread_name, const char *func, @@ -26,51 +24,59 @@ mallctl_thread_name_set_impl(const char *thread_name, const char *func, func, line); mallctl_thread_name_get_impl(thread_name, func, line); } + +#define mallctl_thread_name_get(a) \ + mallctl_thread_name_get_impl(a, __func__, __LINE__) + #define mallctl_thread_name_set(a) \ mallctl_thread_name_set_impl(a, __func__, __LINE__) TEST_BEGIN(test_prof_thread_name_validation) { - const char *thread_name; - test_skip_if(!config_prof); test_skip_if(opt_prof_sys_thread_name); mallctl_thread_name_get(""); - mallctl_thread_name_set("hi there"); + + const char *test_name1 = "test case1"; + mallctl_thread_name_set(test_name1); + + /* Test name longer than the max len. */ + char long_name[] = + "test case longer than expected; test case longer than expected"; + expect_zu_gt(strlen(long_name), PROF_THREAD_NAME_MAX_LEN, + "Long test name not long enough"); + const char *test_name_long = long_name; + expect_d_eq(mallctl("thread.prof.name", NULL, NULL, + (void *)&test_name_long, sizeof(test_name_long)), 0, + "Unexpected mallctl failure from thread.prof.name"); + /* Long name cut to match. */ + long_name[PROF_THREAD_NAME_MAX_LEN - 1] = '\0'; + mallctl_thread_name_get(test_name_long); /* NULL input shouldn't be allowed. */ - thread_name = NULL; + const char *test_name2 = NULL; expect_d_eq(mallctl("thread.prof.name", NULL, NULL, - (void *)&thread_name, sizeof(thread_name)), EFAULT, - "Unexpected mallctl result writing \"%s\" to thread.prof.name", - thread_name); + (void *)&test_name2, sizeof(test_name2)), EINVAL, + "Unexpected mallctl result writing to thread.prof.name"); /* '\n' shouldn't be allowed. */ - thread_name = "hi\nthere"; + const char *test_name3 = "test\ncase"; expect_d_eq(mallctl("thread.prof.name", NULL, NULL, - (void *)&thread_name, sizeof(thread_name)), EFAULT, + (void *)&test_name3, sizeof(test_name3)), EINVAL, "Unexpected mallctl result writing \"%s\" to thread.prof.name", - thread_name); + test_name3); /* Simultaneous read/write shouldn't be allowed. */ - { - const char *thread_name_old; - size_t sz; - - sz = sizeof(thread_name_old); - expect_d_eq(mallctl("thread.prof.name", - (void *)&thread_name_old, &sz, (void *)&thread_name, - sizeof(thread_name)), EPERM, - "Unexpected mallctl result writing \"%s\" to " - "thread.prof.name", thread_name); - } + const char *thread_name_old; + size_t sz = sizeof(thread_name_old); + expect_d_eq(mallctl("thread.prof.name", (void *)&thread_name_old, &sz, + (void *)&test_name1, sizeof(test_name1)), EPERM, + "Unexpected mallctl result from thread.prof.name"); mallctl_thread_name_set(""); } TEST_END -#define NTHREADS 4 -#define NRESET 25 static void * thd_start(void *varg) { unsigned thd_ind = *(unsigned *)varg; @@ -82,6 +88,7 @@ thd_start(void *varg) { mallctl_thread_name_get(""); mallctl_thread_name_set(thread_name); +#define NRESET 25 for (i = 0; i < NRESET; i++) { expect_d_eq(mallctl("prof.reset", NULL, NULL, NULL, 0), 0, "Unexpected error while resetting heap profile data"); @@ -92,12 +99,14 @@ thd_start(void *varg) { mallctl_thread_name_set(""); return NULL; +#undef NRESET } TEST_BEGIN(test_prof_thread_name_threaded) { test_skip_if(!config_prof); test_skip_if(opt_prof_sys_thread_name); +#define NTHREADS 4 thd_t thds[NTHREADS]; unsigned thd_args[NTHREADS]; unsigned i; @@ -109,10 +118,9 @@ TEST_BEGIN(test_prof_thread_name_threaded) { for (i = 0; i < NTHREADS; i++) { thd_join(thds[i], NULL); } +#undef NTHREADS } TEST_END -#undef NTHREADS -#undef NRESET int main(void) {