Repository navigation
Conversation
test_configs.py turns every warning into an error. A replay buffer and
its storage reference each other, so a dropped buffer is freed by the
cyclic GC, not right away. Its storage and writer hold mp.Value
semaphores. When the GC runs while a new semaphore is being registered
with the multiprocessing resource tracker, the old semaphore's
finalizer calls back into the tracker. CPython refuses the reentrant
call and emits a UserWarning ("ResourceTracker called reentrantly").
The error filter turns that into an unraisable exception, and pytest
fails whatever test happened to be running.
Ignore that one warning in the module so the error filter keeps
catching everything else.
Signed-off-by: Ankit Jha <jhaankit373@gmail.com>
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/rl/4718
Note: Links to docs will display an error until the docs builds have been completed.
|
|
Hi @ankit373! Thank you for your pull request and welcome to our community. Action RequiredIn order to merge any pull request (code, docs, etc.), we require contributors to sign our Contributor License Agreement, and we don't seem to have one on file for you. ProcessIn order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA. Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with If you have received this in error or have any questions, please contact us at cla@meta.com. Thanks! |
meta-cla
Bot
added
the
CLA Signed
|
Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Meta Open Source project. Thanks! |
Description
fixes the cpu flake
test/test_configs.py::TestDataConfigs::test_complex_replay_buffer_configurationfrom the flaky report. the cuda dreamer test in the same report is not touched here.the test itself is fine. it fails from a gc side effect:
test_configs.pysetspytestmark = pytest.mark.filterwarnings("error").storage.attach(buffer)), so a dropped buffer is freed by the cyclic gc, not by refcount. its storage and writer holdmp.Valuesemaphores.resource_tracker.unregisterinside the tracker's own lock. cpython refuses the reentrant call and emitsUserWarning: ResourceTracker called reentrantly for resource cleanup ... might leak.ExceptionGroup: multiple unraisable exception warnings (6 sub-exceptions), each oneReentrantCallError-> thatUserWarning(e.g. tests-cpu-mid (3.12, bulk), run 42443177736).the fix ignores only that warning in the module. every other warning still fails the test (checked that a plain
UserWarningstill errors under the newpytestmark).Motivation and Context
Fixes #4480 (only the
test_complex_replay_buffer_configurationpart)flake rate, macos arm64, python 3.12.13, torch 2.15.0 cpu, tensordict 0.15.5, pytest 9.2.0, hydra-core 1.3.8 (same pytest/hydra as ci):
TestDataConfigsclass--count 300(9900 runs)all "before" failures had the same
ReentrantCallError/ResourceTracker called reentrantlysignature as ci. the fulltest_configs.pyrun has the same set of local failures (missing optional deps) before and after.side note, not changed here: the storage -> buffer strong ref in
Storage.attachis what keeps buffers alive until the cyclic gc. a weakref there would let buffers be freed right away, but it touches pickling and sharing, so it felt out of scope for a test fix.Types of changes
Checklist