Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUG] Realm::RuntimeImpl::network_init() leaks argv #1754

Open
Jacobfaib opened this issue Sep 5, 2024 · 3 comments
Open

[BUG] Realm::RuntimeImpl::network_init() leaks argv #1754

Jacobfaib opened this issue Sep 5, 2024 · 3 comments

Comments

@Jacobfaib
Copy link
Contributor

Jacobfaib commented Sep 5, 2024

Direct leak of 40 byte(s) in 1 object(s) allocated from:
    #0 0x102aee510 in malloc (/opt/homebrew/Cellar/llvm/18.1.8/lib/clang/18/lib/darwin/libclang_rt.asan_osx_dynamic.dylib:arm64+0x52510)
    #1 0x1331ae49c in Realm::RuntimeImpl::network_init(int*, char***) /path/to/runtime/realm/runtime_impl.cc:1330:46
int new_argc = local_argc + starts.size();
const char **new_argv = (const char **)(malloc((new_argc + 1) * sizeof(char *))); // <--- never free'd
// ...

This also leads to indirect leak of

Indirect leak of 18 byte(s) in 2 object(s) allocated from:
    #0 0x102aee510 in malloc (/opt/homebrew/Cellar/llvm/18.1.8/lib/clang/18/lib/darwin/libclang_rt.asan_osx_dynamic.dylib:arm64+0x52510)
    #1 0x102ab2e50 in strndup (/opt/homebrew/Cellar/llvm/18.1.8/lib/clang/18/lib/darwin/libclang_rt.asan_osx_dynamic.dylib:arm64+0x16e50)
    #2 0x1331aeaf0 in Realm::RuntimeImpl::network_init(int*, char***) /path/to/runtime/realm/runtime_impl.cc:1341:35
for(size_t i = 0; i < starts.size(); i++)
  new_argv[i + before_new] = strndup(starts[i], ends[i] - starts[i]); // <---
@muraj
Copy link

muraj commented Sep 5, 2024

This one is a bit more difficult to deal with. Unfortunately, for some reason, we set the passed in argc and argv to the newly allocated local_argc and local_argv. So the caller actually depends on the newly allocated argv and of course it assumes it doesn't need to release it, which leaks.

This is yet another reason why I highly dislike relying on command line arguments for configuring Realm, and I am hoping the path for unifying the processors (and eventually the memories) will reduce and eventually eliminate the need to specify command line arguments to realm at all. Until then, this leak will likely have to persist...

@eddy16112
Copy link
Contributor

As long as we do not use REALM_DEFAULT_ARGS, we should not have leaks.

@Jacobfaib
Copy link
Contributor Author

Legate exclusively uses REALM_DEFAULT_ARGS instead of argc and argv. IIRC we do forward them along (in case the user has put something in there), but we do not populate them.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants