[zanieb]

The LockFile implementation opens a file descriptor to check locks. In the non-blocking case, if the lock cannot be acquired, a LockError is thrown and an open file descriptor is left dangling. In normal operation, this file is closed on lock release, but since the lock has already been acquired by another instance of the LockFile, release never occurs. On teardown, a ResourceWarning is thrown by the Python runtime e.g.

E               Traceback (most recent call last):
E                 File "/opt/hostedtoolcache/Python/3.8.12/x64/lib/python3.8/site-packages/distributed/diskutils.py", line 173, in _purge_leftovers
E                   if self._check_lock_or_purge(path):
E               ResourceWarning: unclosed file <_io.BufferedWriter name='/home/runner/work/orion/orion/dask-worker-space/worker-ie4t4hva.dirlock'>

I discovered this in teardown calls to distributed.diskutils::WorkSpace._check_lock_or_purge as you can see above.

This implementation drops caching of the _file descriptor on the LockFile object. An alternate implementation could retain the caching but close the file and set it to None on exception during acquire.

This diff is best viewed with whitespace changes ignored https://github.com/dask/distributed/pull/6122/files?w=1

• Closes Resource Warnings with LocalCluster  #2486
• Tests added / passed
• Passes pre-commit run --all-files

[zanieb]

I've noticed this is vendored, perhaps the comment at the top deserves an update here as well.

[mrocklin]

This seems sensible to me. Thanks @madkinsz

[mrocklin]

I'm open to unvendoring this as well (I honestly don't recall the motivation there, maybe it wasn't yet published at the time?) Do you have a recommendation here?

[zanieb]

Great, thanks for taking a look! I'll dig into these test failures and mark as ready for review once it's sorted out.

[zanieb]

I'm open to unvendoring this as well (I honestly don't recall the motivation there, maybe it wasn't yet published at the time?) Do you have a recommendation here?

It looks like this is vendored based on mwilliamson/locket.py#8 to gain some windows optimizations but it's from 2017? There's a 2021 release but all it seems to do is clean up publishing / administrative tasks. I can see if that PR is going to be merged and open this as an issue there to see if it's being actively maintained.

[zanieb]

This seems less than ideal, but if someone has already purged the worker directory this lock file is gone and we'll throw an exception here without checking if the path exists.

I originally included a warning but it is reraised as an error during Dask's test run and we'd have to ignore it or avoid calling release after the work space has been deleted.

Suggested change

	if os.path.exists(self._path):
	with open(self._path, "wb") as fd:
	_unlock_file(fd)
	if os.path.exists(self._path):
	with open(self._path, "wb") as fd:
	_unlock_file(fd)
	else:
	warnings.warn(f"Lock at {self._path!r} no longer exists and cannot be released.", stacklevel=2)

[github-actions]

Unit Test Results

       16 files  +    4           3 errors    13 suites  +1   5h 24m 13s ⏱️ - 1m 17s
  2 738 tests +    4    2 292 ✔️  - 351    68 💤  - 22    6 ❌ +  6     372 🔥 +   371 
17 206 runs  +845  15 356 ✔️  - 203  722 💤  - 79  21 ❌ +21  1 107 🔥 +1 106 

For more details on these parsing errors, failures and errors, see this check.

Results for commit a37b87e. ± Comparison against base commit 7d55039.

[zanieb]

I'm going to contribute upstream and open another pull request unvendoring locket as the maintainer is active and responsive :)