From 03626abc3edef9672e9bc134f4622b013d1fdebd Mon Sep 17 00:00:00 2001 From: Olivier Grisel Date: Mon, 7 Feb 2022 15:29:23 +0100 Subject: [PATCH 1/7] WIP experimenting with resource_tracker warning failures --- joblib/_memmapping_reducer.py | 4 ++++ joblib/test/test_memmapping.py | 15 ++++++++++----- 2 files changed, 14 insertions(+), 5 deletions(-) diff --git a/joblib/_memmapping_reducer.py b/joblib/_memmapping_reducer.py index d58382222..9d350c032 100644 --- a/joblib/_memmapping_reducer.py +++ b/joblib/_memmapping_reducer.py @@ -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 diff --git a/joblib/test/test_memmapping.py b/joblib/test/test_memmapping.py index f110751fb..32991205d 100644 --- a/joblib/test/test_memmapping.py +++ b/joblib/test/test_memmapping.py @@ -679,7 +679,7 @@ 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 @@ -687,8 +687,11 @@ def test_resource_tracker_silent_when_reference_cycles(backend): # 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. + # # Note that the script in ``cmd`` is the exact same script as in # test_permission_error_windows_reference_cycle. cmd = """if 1: @@ -714,8 +717,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, err or out + assert "resource_tracker" not in err, err @with_numpy From 2401b9f57e6f54ed8f36e414b6532e2c4a9dc5d7 Mon Sep 17 00:00:00 2001 From: Olivier Grisel Date: Mon, 7 Feb 2022 16:07:57 +0100 Subject: [PATCH 2/7] try to make test_child_raises_parent_exits_cleanly pass on macos --- joblib/test/test_memmapping.py | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/joblib/test/test_memmapping.py b/joblib/test/test_memmapping.py index 32991205d..7b74f7612 100644 --- a/joblib/test/test_memmapping.py +++ b/joblib/test/test_memmapping.py @@ -822,8 +822,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 From 149521fdf73031958294b548efcfab32a062e59f Mon Sep 17 00:00:00 2001 From: Olivier Grisel Date: Mon, 7 Feb 2022 16:08:55 +0100 Subject: [PATCH 3/7] Lower the probability of not collecting a temp folder on Windows in the presence of ref cycles --- joblib/_memmapping_reducer.py | 2 +- joblib/disk.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/joblib/_memmapping_reducer.py b/joblib/_memmapping_reducer.py index 9d350c032..7db967736 100644 --- a/joblib/_memmapping_reducer.py +++ b/joblib/_memmapping_reducer.py @@ -661,7 +661,7 @@ def _try_delete_folder(self, allow_non_empty, context_id=None): # Now that this folder is deleted, we can forget about it self._unregister_context(context_id) - except OSError: + except PermissionError: # Temporary folder cannot be deleted right now. No need to # handle it though, as this folder will be cleaned up by an # atexit finalizer registered by the memmapping_reducer. diff --git a/joblib/disk.py b/joblib/disk.py index 1bf0756d0..32fbb89f6 100644 --- a/joblib/disk.py +++ b/joblib/disk.py @@ -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): From 01d8959bd6a09f0a37885f029a485243ac8c3e6d Mon Sep 17 00:00:00 2001 From: Olivier Grisel Date: Mon, 7 Feb 2022 16:36:21 +0100 Subject: [PATCH 4/7] Did not mean to change this --- joblib/_memmapping_reducer.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/joblib/_memmapping_reducer.py b/joblib/_memmapping_reducer.py index 7db967736..9d350c032 100644 --- a/joblib/_memmapping_reducer.py +++ b/joblib/_memmapping_reducer.py @@ -661,7 +661,7 @@ def _try_delete_folder(self, allow_non_empty, context_id=None): # Now that this folder is deleted, we can forget about it self._unregister_context(context_id) - except PermissionError: + except OSError: # Temporary folder cannot be deleted right now. No need to # handle it though, as this folder will be cleaned up by an # atexit finalizer registered by the memmapping_reducer. From 230ceac434e08aea0f578f1efa1e5b181b6bffea Mon Sep 17 00:00:00 2001 From: Olivier Grisel Date: Mon, 7 Feb 2022 17:58:51 +0100 Subject: [PATCH 5/7] xfail only the refcycle + windows case --- joblib/test/test_memmapping.py | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/joblib/test/test_memmapping.py b/joblib/test/test_memmapping.py index 7b74f7612..d3ed46825 100644 --- a/joblib/test/test_memmapping.py +++ b/joblib/test/test_memmapping.py @@ -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 @@ -690,10 +692,19 @@ def test_resource_tracker_silent_when_reference_cycles(backend): # confusing messages. # # This test makes sure that the resource_tracker is silent when a reference - # has been collected concurrently. + # 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 @@ -719,7 +730,7 @@ def test_resource_tracker_silent_when_reference_cycles(backend): out, err = p.communicate() out = out.decode() err = err.decode() - assert p.returncode == 0, err or out + assert p.returncode == 0, out + "\n\n" + err assert "resource_tracker" not in err, err From 02dfec5117a6b4681e5c98c15872fcbe9359e4c7 Mon Sep 17 00:00:00 2001 From: Olivier Grisel Date: Mon, 7 Feb 2022 18:34:23 +0100 Subject: [PATCH 6/7] Trigger CI again From bcd2aaf542164abfffcc21986d321257c581d242 Mon Sep 17 00:00:00 2001 From: Olivier Grisel Date: Mon, 7 Feb 2022 19:09:33 +0100 Subject: [PATCH 7/7] Document fix --- CHANGES.rst | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGES.rst b/CHANGES.rst index bff605c94..8b3d7e257 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -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 --------------