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

Fix remaining resource_tracker-related test warning failures #1263

Merged
merged 7 commits into from Feb 7, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGES.rst
Expand Up @@ -4,6 +4,10 @@ Latest changes
Development version
-------------------

- Avoid unnecessary warnings when workers and main process delete
the temporary memmap folder contents concurrently.
https://github.com/joblib/joblib/pull/1263

Release 1.1.0
--------------

Expand Down
4 changes: 4 additions & 0 deletions joblib/_memmapping_reducer.py
Expand Up @@ -99,6 +99,10 @@ def unlink_file(filename):
raise
else:
time.sleep(.2)
except FileNotFoundError:
# In case of a race condition when deleting the temporary folder,
# avoid noisy FileNotFoundError exception in the resource tracker.
pass


resource_tracker._CLEANUP_FUNCS['file'] = unlink_file
Expand Down
2 changes: 1 addition & 1 deletion joblib/disk.py
Expand Up @@ -69,7 +69,7 @@ def mkdirp(d):
# exception. this mechanism ensures that the sub-process gc have the time to
# collect and close the memmaps before we fail.
RM_SUBDIRS_RETRY_TIME = 0.1
RM_SUBDIRS_N_RETRY = 5
RM_SUBDIRS_N_RETRY = 10


def rm_subdirs(path, onerror=None):
Expand Down
35 changes: 28 additions & 7 deletions joblib/test/test_memmapping.py
Expand Up @@ -9,6 +9,8 @@
import subprocess
import threading

import pytest

from joblib.test.common import with_numpy, np
from joblib.test.common import setup_autokill
from joblib.test.common import teardown_autokill
Expand Down Expand Up @@ -679,18 +681,30 @@ def test_memmap_returned_as_regular_array(backend):

@with_numpy
@with_multiprocessing
@parametrize("backend", ["multiprocessing", param("loky", marks=xfail)])
@parametrize("backend", ["multiprocessing", "loky"])
def test_resource_tracker_silent_when_reference_cycles(backend):
# There is a variety of reasons that can make joblib with loky backend
# output noisy warnings when a reference cycle is preventing a memmap from
# being garbage collected. Especially, joblib's main process finalizer
# deletes the temporary folder if it was not done before, which can
# interact badly with the resource_tracker. We don't risk leaking any
# resources, but this will likely make joblib output a lot of low-level
# confusing messages. This test is marked as xfail for now: but a next PR
# should fix this behavior.
# confusing messages.
#
# This test makes sure that the resource_tracker is silent when a reference
# has been collected concurrently on non-Windows platforms.
#
# Note that the script in ``cmd`` is the exact same script as in
# test_permission_error_windows_reference_cycle.
if backend == "loky" and sys.platform.startswith('win'):
# XXX: on Windows, reference cycles can delay timely garbage collection
# and make it impossible to properly delete the temporary folder in the
# main process because of permission errors.
pytest.xfail(
"The temporary folder cannot be deleted on Windows in the "
"presence of a reference cycle"
)

cmd = """if 1:
import numpy as np
from joblib import Parallel, delayed
Expand All @@ -714,8 +728,10 @@ def test_resource_tracker_silent_when_reference_cycles(backend):
stdout=subprocess.PIPE)
p.wait()
out, err = p.communicate()
assert p.returncode == 0, out.decode()
assert b"resource_tracker" not in err, err.decode()
out = out.decode()
err = err.decode()
assert p.returncode == 0, out + "\n\n" + err
assert "resource_tracker" not in err, err


@with_numpy
Expand Down Expand Up @@ -817,8 +833,13 @@ def get_temp_folder(parallel_obj, backend):
for i in range(1))
except ValueError as e:
# the temporary folder should be deleted by the end of this
# call
if os.path.exists(temp_folder):
# call but apparently on some file systems, this takes
# some time to be visible.
for i in range(10):
if not os.path.exists(temp_folder):
break
sleep(.1)
else:
raise AssertionError(
temp_folder + " was not deleted"
) from e
Expand Down