🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/119449

• 📄 Preview Python docs built from this PR
• 📄 Preview C++ docs built from this PR
• ❓ Need help or want to give feedback on the CI? Visit the bot commands wiki or our office hours

Note: Links to docs will display an error until the docs builds have been completed.

❌ 9 New Failures

As of commit 6531b55 with merge base 6e10f7d ():

This comment was automatically generated by Dr. CI and updates every 15 minutes.

I would expect that a user setting something on os.env in python would trigger such a race? That was my assumption here.

I would expect that a user setting something on os.env in python would trigger such a race? That was my assumption here.

Yes, but the current implementation would do nothing to avoid it, as putenv call from CPython would not be serialized against getenv calls made from libtorch

I would expect that a user setting something on os.env in python would trigger such a race? That was my assumption here.

A specific reproducing example may be hard to create. Also note that Torch may be used in a pure multi-threading C++ environment.

@malfet @albanD set_env was added.

Can you please write a better description at what you are trying to achieve here? I haven't read the standard, but being familiar with the implementation details, I don't see why getenv on its own is not thread safe.

It is probably unsafe when there are a concurrent putenv calls, but you are not adding that one to your env utils.

[Edit] To quote https://en.cppreference.com/w/cpp/utility/program/getenv looks like it is part of the standard:

This function is thread-safe (calling it from multiple threads does not introduce a data race) as long as no other function modifies the host environment. In particular, the POSIX functions setenv(), unsetenv(), and putenv() would introduce a data race if called without synchronization.

[Edit2] Is this about suppressing some clang-tidy warnings or solving a real problem? If later, then solution is to provide thread-safe extern "C" implementation of those methods in the code, that would take over libc ones (all libc functions are weak symbols by design so that one can app can choose to override them) and provide thread safety from getenv calls fro say oneDNN, libcuda etc

The purpose is to suppress clang-tidy warnings and solve possible future problems by providing thread-safe getenv and setenv. Overwriting libc implementations may not be a good idea because we have to support MSVC and other no GNU platforms so that I prefer to use explicit c10 API.

@pytorchbot label "topic: not user facing"

@pytorchbot label ciflow/trunk

Looks like this PR hasn't been updated in a while so we're going to go ahead and mark this as Stale.
Feel free to remove the Stale label if you feel this was a mistake.
If you are unable to remove the Stale label please contact a maintainer in order to do so.
If you want the bot to never mark this PR stale again, add the no-stale label.
Stale pull requests will automatically be closed after 30 days of inactivity.

commenting that this is a real issue we've seen in practice pytorch/kineto#984

but problematic interactions might be more likely between PyTorch and third party libraries as above

@pytorchbot rebase

@pytorchbot started a rebase job onto refs/remotes/origin/viable/strict. Check the current status here

Successfully rebased th_safe_env onto refs/remotes/origin/viable/strict, please pull locally before adding more changes (for example, via git checkout th_safe_env && git pull --rebase)

Rewrite code to avoid changing existing API.

@pytorchmergebot merge

Merge started

Your change will be merged once all checks pass (ETA 0-4 Hours).

Learn more about merging in the wiki.

Questions? Feedback? Please reach out to the PyTorch DevX Team

Merge failed

Reason: 5 jobs have failed, first few of them are: macos-arm64-binary-wheel / wheel-py3_9-cpu-build, macos-arm64-binary-wheel / wheel-py3_10-cpu-build, macos-arm64-binary-wheel / wheel-py3_12-cpu-build, macos-arm64-binary-wheel / wheel-py3_11-cpu-build, macos-arm64-binary-libtorch-cxx11-abi / libtorch-cpu-shared-with-deps-cxx11-abi-build

@pytorchbot rebase -b main

@pytorchbot started a rebase job onto refs/remotes/origin/main. Check the current status here

Successfully rebased th_safe_env onto refs/remotes/origin/main, please pull locally before adding more changes (for example, via git checkout th_safe_env && git pull --rebase)

@pytorchbot merge -f "Unrelated failures"

Merge started

Your change will be merged immediately since you used the force (-f) flag, bypassing any CI checks (ETA: 1-5 minutes). Please use -f as last resort and instead consider -i/--ignore-current to continue the merge ignoring current failures. This will allow currently pending tests to finish and report signal before the merge.

Learn more about merging in the wiki.

Questions? Feedback? Please reach out to the PyTorch DevX Team