Use global base_alloc in proxy lib #263
Conversation
igchor
force-pushed
the
only_global_ba_alloc
branch
4 times, most recently
from
77cf244 to
a1bd5c4
Compare
global base alloc free function now does not require size param
- 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).
igchor
force-pushed
the
only_global_ba_alloc
branch
from
a1bd5c4 to
c0c35c4
Compare
src/base_alloc/base_alloc_linux.c
Outdated
| @@ -33,7 +35,8 @@ void *ba_os_alloc(size_t size) { | |||
|
|
|||
| void ba_os_free(void *ptr, size_t size) { | |||
| int ret = munmap(ptr, size); | |||
| assert(ret == 0); | |||
| // TODO: this might fail in proxy_lib | |||
There was a problem hiding this comment.
This change is applied here and reverted in the next commit. Remove it at all, please.
| @@ -35,8 +37,7 @@ void *ba_os_alloc(size_t size) { | |||
|
|
|||
| void ba_os_free(void *ptr, size_t size) { | |||
| int ret = munmap(ptr, size); | |||
| // TODO: this might fail in proxy_lib | |||
There was a problem hiding this comment.
This change was applied in the previous commit and is reverted here. Remove it at all, please.
| umfPoolDestroy(Proxy_pool); | ||
| Proxy_pool = NULL; | ||
|
|
||
| umfMemoryProviderDestroy(OS_memory_provider); | ||
| OS_memory_provider = NULL; | ||
|
|
||
| umf_ba_destroy_global(); |
There was a problem hiding this comment.
This change was applied in the previous commit and is reverted here. Remove it at all, please.
src/proxy_lib/proxy_lib.c
Outdated
| umfMemoryProviderDestroy(OS_memory_provider); | ||
| OS_memory_provider = NULL; | ||
|
|
||
| umf_ba_destroy_global(); |
There was a problem hiding this comment.
This change is applied here and reverted in the next commit. Remove it at all, please.
There was a problem hiding this comment.
I mean adding umf_ba_destroy_global();
| @@ -93,13 +93,12 @@ void proxy_lib_create_common(void) { | |||
| } | |||
|
|
|||
| void proxy_lib_destroy_common(void) { | |||
|
|
|||
| return user_ptr; | ||
| } | ||
|
|
||
| static void *get_original_alloc(void *user_ptr, size_t *total_size, |
There was a problem hiding this comment.
Add a comment what this function does, please.
|
|
||
| void *user_ptr; | ||
| if (alignment <= sizeof(size_t)) { | ||
| user_ptr = (void *)((uintptr_t)ptr + sizeof(size_t)); |
There was a problem hiding this comment.
Add a comment why it is aligned to alignment, please.
| #include "proxy_lib.h" | ||
| #include "utils_common.h" | ||
| #include "utils_concurrency.h" | ||
|
|
||
| /* | ||
| * The UMF proxy library uses two memory allocators: | ||
| * 1) the "LEAK" internal linear base allocator based on the anonymous mapped | ||
| * memory that will NOT be destroyed (with API ba_leak_*()). | ||
| * 1) base_alloc for bootstrapping |
| } | ||
|
|
||
| if (Proxy_pool) { | ||
| if (!was_called_from_umfPool && Proxy_pool) { |
There was a problem hiding this comment.
(Important) What if an app wants to realloc a pointer from the base allocator when Proxy_pool already exists?
How can you distinguish which allocator a pointer comes from?
| } | ||
|
|
||
| if (Proxy_pool) { | ||
| if (!was_called_from_umfPool && Proxy_pool) { |
There was a problem hiding this comment.
(Important) What if an app wants to free a pointer from the base allocator when Proxy_pool already exists?
How can you distinguish which allocator a pointer comes from?
There was a problem hiding this comment.
General comment:
This PR merges 4 independent changes:
- Fixing a 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).)
- Improvements in the global base allocator (adding metadata with stored size,
alloc_usable_size()and removing size fromfree()) - Replacing the linear base allocator with the global base allocator in the proxy library
- Removing the linear base allocator from the repo.
IMHO this PR should be split into 4 separate smaller pull requests.
There was a problem hiding this comment.
$ LD_PRELOAD=./lib/libumf_proxy.so ctest --output-on-failureends with Segmentation fault:
Program received signal SIGSEGV, Segmentation fault.
0x00007ffff7833bf5 in __GI_getenv (name=0x7ffff694eedc "GNUTLS_NO_IMPLICIT_INIT") at ./stdlib/getenv.c:75
75 for (ep = __environ; *ep != NULL; ++ep)
|
@igchor please split this change into multiple PRs |
Replace linear allocator with global base alloc. This PR also makes linear allocator obsolete and hence fixes: #260
After these changes, it is possible to destroy Proxy_pool and OS provider in proxy lib.
During debugging I found that there is still one or two allocations coming from dlopen that are not free, but they are bigger than any allocation class size that we use hence it doesn't trigger any asserts.