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

UCM: Avoid releasing environment strings in library destructor - v1.6.x #3446

Merged
merged 1 commit into from
Apr 4, 2019

Conversation

yosefe
Copy link
Contributor

@yosefe yosefe commented Apr 4, 2019

No description provided.

Since now libucm.so is not linked with "nodelete", it may be unloaded
before it installs malloc hooks. In this case we would not have any
environment strings, so no need to release. If it did install hooks,
it means the library was aready reopened with RTLD_NODELETE, so no need
the strings anyway.

This fixes a case where libucm.so was unloaded before hooks were
installed, and called clearenv() which removed all environment variables
from the program, leading to segfault.

Since we are not clearing the environment now, we also cannot release
the allocated strings. This is not reported as memory leak by Valgrind,
since they are still reachable from the global array of environ strings.
@yosefe yosefe changed the title UCM: Avoid releasing environment strings in library destructor UCM: Avoid releasing environment strings in library destructor - v1.6.x Apr 4, 2019
@swx-jenkins1
Copy link

Test PASSed.
See http://bgate.mellanox.com/jenkins/job/gh-ucx-pr/6865/ for details.

@mellanox-github
Copy link
Contributor

Test PASSed.
See http://hpc-master.lab.mtl.com:8080/job/hpc-ucx-pr/9655/ for details (Mellanox internal link).

@yosefe
Copy link
Contributor Author

yosefe commented Apr 4, 2019

@hoopoepg @brminich can you pls take a look?

@yosefe yosefe merged commit 12ce221 into openucx:v1.6.x Apr 4, 2019
@yosefe yosefe added the Bugfix label Apr 4, 2019
@yosefe yosefe deleted the topic/ucm-fix-env-cleanup-v1.6.x branch May 26, 2019 10:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants