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

Implement more stringent thread-safetyness in rcutils_getenv #242

Open
clalancette opened this issue Apr 27, 2020 · 2 comments
Open

Implement more stringent thread-safetyness in rcutils_getenv #242

clalancette opened this issue Apr 27, 2020 · 2 comments
Labels
backlog enhancement New feature or request

Comments

@clalancette
Copy link
Contributor

There's a lot of detail in #237 (comment) , but to summarize the situation we are now in:

As of #237, on all platforms rcutils_get_env is thread-safe for simultaneously getting environment variables from separate threads. It is currently unsafe in the following cases:

  1. Getting an environment variable, holding onto the pointer, and then having a later method (in the same or different thread) call setenv. In that case, the pointer may be invalidated, but there is no way of knowing.
  2. Getting an environment variable in one thread while setting an environment variable from a separate thread at the same time (this is a well-known limitation of glibc, for instance).

The first issue can be solved by changing the contract of rcutils_get_env to take an allocator, allocate space, copy the contents of the environment variable into that space, and then having the caller free the memory when they are done.

The second issue can be solved by adding locking around getting environment variables and setting environment variables.

@hidmic
Copy link
Contributor

hidmic commented Apr 30, 2020

The first issue can be solved by changing the contract of rcutils_get_env to take an allocator, allocate space, copy the contents of the environment variable into that space, and then having the caller free the memory when they are done.

👍

The second issue can be solved by adding locking around getting environment variables and setting environment variables.

Client code, or the libraries it pulls in, can always call setenv or putenv avoiding that lock. So I don't think we can do much about that.

@clalancette
Copy link
Contributor Author

Client code, or the libraries it pulls in, can always call setenv or putenv avoiding that lock. So I don't think we can do much about that.

Yeah, that's a good point. "Solved" is the wrong term here; "mitigated in certain circumstances" is more correct. Luckily, doing setenv/putenv isn't a common operation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backlog enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants