Skip to content

Commit 77cf244

Browse files
committed
Fix problem with freeing memory in proxy lib
- make sure base_alloc dtor is only called once by adding a macro to skip the dtor (it's set in proxy lib). Previously, since base_alloc was linked with both libumf and proxy_lib, it was called twice and the first time was BEFORE the libumf dtor (hence some allocations were still unfreed).
1 parent c5ec9bb commit 77cf244

File tree

8 files changed

+25
-54
lines changed

8 files changed

+25
-54
lines changed

src/base_alloc/base_alloc.c

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -238,13 +238,6 @@ void umf_ba_free(umf_ba_pool_t *pool, void *ptr) {
238238
}
239239

240240
void umf_ba_destroy(umf_ba_pool_t *pool) {
241-
// Do not destroy if we are running in the proxy library,
242-
// because it may need those resources till
243-
// the very end of exiting the application.
244-
if (pool->metadata.n_allocs && is_running_in_proxy_lib()) {
245-
return;
246-
}
247-
248241
#ifndef NDEBUG
249242
ba_debug_checks(pool);
250243
if (pool->metadata.n_allocs) {

src/base_alloc/base_alloc_linux.c

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,9 +24,11 @@ static size_t Page_size;
2424
// as the first one and the destructor as the last one in order to avoid use-after-free.
2525
void __attribute__((constructor(101))) umf_ba_constructor(void) {}
2626

27+
#ifndef SKIP_BASE_ALLOC_DTOR
2728
void __attribute__((destructor(101))) umf_ba_destructor(void) {
2829
umf_ba_destroy_global();
2930
}
31+
#endif
3032

3133
void *ba_os_alloc(size_t size) {
3234
return mmap(NULL, size, PROT_READ | PROT_WRITE, MAP_PRIVATE | MAP_ANONYMOUS,
@@ -35,8 +37,7 @@ void *ba_os_alloc(size_t size) {
3537

3638
void ba_os_free(void *ptr, size_t size) {
3739
int ret = munmap(ptr, size);
38-
// TODO: this might fail in proxy_lib
39-
// assert(ret == 0);
40+
assert(ret == 0);
4041
(void)ret; // unused
4142
}
4243

src/libumf_linux.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,11 +15,11 @@
1515

1616
umf_memory_tracker_handle_t TRACKER = NULL;
1717

18-
void __attribute__((constructor)) umfCreate(void) {
18+
void __attribute__((constructor(102))) umfCreate(void) {
1919
TRACKER = umfMemoryTrackerCreate();
2020
}
2121

22-
void __attribute__((destructor)) umfDestroy(void) {
22+
void __attribute__((destructor(102))) umfDestroy(void) {
2323
umf_memory_tracker_handle_t t = TRACKER;
2424
// make sure TRACKER is not used after being destroyed
2525
TRACKER = NULL;

src/provider/provider_tracking.c

Lines changed: 11 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -361,20 +361,18 @@ static void check_if_tracker_is_empty(umf_memory_tracker_handle_t hTracker,
361361
// Do not assert if we are running in the proxy library,
362362
// because it may need those resources till
363363
// the very end of exiting the application.
364-
if (!is_running_in_proxy_lib()) {
365-
if (pool) {
366-
fprintf(stderr,
367-
"ASSERT: tracking provider of pool %p is not empty! "
368-
"(%zu items left)\n",
369-
(void *)pool, n_items);
370-
} else {
371-
fprintf(stderr,
372-
"ASSERT: tracking provider is not empty! (%zu items "
373-
"left)\n",
374-
n_items);
375-
}
376-
assert(n_items == 0);
364+
if (pool) {
365+
fprintf(stderr,
366+
"ASSERT: tracking provider of pool %p is not empty! "
367+
"(%zu items left)\n",
368+
(void *)pool, n_items);
369+
} else {
370+
fprintf(stderr,
371+
"ASSERT: tracking provider is not empty! (%zu items "
372+
"left)\n",
373+
n_items);
377374
}
375+
assert(n_items == 0);
378376
}
379377
}
380378
#endif /* NDEBUG */
@@ -511,13 +509,6 @@ void umfMemoryTrackerDestroy(umf_memory_tracker_handle_t handle) {
511509
return;
512510
}
513511

514-
// Do not destroy if we are running in the proxy library,
515-
// because it may need those resources till
516-
// the very end of exiting the application.
517-
if (is_running_in_proxy_lib()) {
518-
return;
519-
}
520-
521512
#ifndef NDEBUG
522513
check_if_tracker_is_empty(handle, NULL);
523514
#endif /* NDEBUG */

src/proxy_lib/CMakeLists.txt

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,9 +39,9 @@ add_umf_library(NAME umf_proxy
3939
add_library(${PROJECT_NAME}::proxy ALIAS umf_proxy)
4040

4141
if(PROXY_LIB_USES_SCALABLE_POOL)
42-
target_compile_definitions(umf_proxy PRIVATE PROXY_LIB_USES_SCALABLE_POOL=1)
42+
target_compile_definitions(umf_proxy PRIVATE PROXY_LIB_USES_SCALABLE_POOL=1 SKIP_BASE_ALLOC_DTOR=1)
4343
elseif(PROXY_LIB_USES_JEMALLOC_POOL)
44-
target_compile_definitions(umf_proxy PRIVATE PROXY_LIB_USES_JEMALLOC_POOL=1)
44+
target_compile_definitions(umf_proxy PRIVATE PROXY_LIB_USES_JEMALLOC_POOL=1 SKIP_BASE_ALLOC_DTOR=1)
4545
endif()
4646

4747
target_include_directories(umf_proxy PUBLIC

src/proxy_lib/proxy_lib.c

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -93,13 +93,12 @@ void proxy_lib_create_common(void) {
9393
}
9494

9595
void proxy_lib_destroy_common(void) {
96+
9697
umfPoolDestroy(Proxy_pool);
9798
Proxy_pool = NULL;
9899

99100
umfMemoryProviderDestroy(OS_memory_provider);
100101
OS_memory_provider = NULL;
101-
102-
umf_ba_destroy_global();
103102
}
104103

105104
/*****************************************************************************/

src/proxy_lib/proxy_lib_linux.c

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -9,15 +9,14 @@
99

1010
#include "proxy_lib.h"
1111

12-
// The priority 102 is used, because the constructor should be called as the second one
13-
// (just after the first constructor of the base allocator with priority 101)
14-
// and the destructor as the last but one (just before the last destructor
15-
// of the base allocator with priority 101), because this library
16-
// provides the memory allocation API.
17-
void __attribute__((constructor(102))) proxy_lib_create(void) {
12+
// Priority forces this ctor to be called after both libumf and
13+
// base_alloc ctors were called
14+
void __attribute__((constructor(103))) proxy_lib_create(void) {
1815
proxy_lib_create_common();
1916
}
2017

21-
void __attribute__((destructor(102))) proxy_lib_destroy(void) {
18+
// Priority forces this dtor to be called before both libumf and
19+
// base_alloc dtors were called
20+
void __attribute__((destructor(103))) proxy_lib_destroy(void) {
2221
proxy_lib_destroy_common();
2322
}

src/utils/utils_common.h

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -65,18 +65,6 @@ static inline size_t util_get_page_size(void) { return sysconf(_SC_PAGE_SIZE); }
6565

6666
#endif /* _WIN32 */
6767

68-
// check if we are running in the proxy library
69-
static inline int is_running_in_proxy_lib(void) {
70-
int is_in_proxy_lib_val = 0;
71-
char *ld_preload = util_getenv("LD_PRELOAD");
72-
if (ld_preload && strstr(ld_preload, "libumf_proxy.so")) {
73-
is_in_proxy_lib_val = 1;
74-
}
75-
76-
util_free_getenv(ld_preload);
77-
return is_in_proxy_lib_val;
78-
}
79-
8068
#define NOFUNCTION \
8169
do { \
8270
} while (0)

0 commit comments

Comments
 (0)