[issue35144] TemporaryDirectory clean-up fails with unsearchable directories

[Eryk Sun]

Change by Eryk Sun :


--
versions: +Python 3.10, Python 3.8, Python 3.9 -Python 3.7

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue35144] TemporaryDirectory clean-up fails with unsearchable directories

[Eryk Sun]

Eryk Sun  added the comment:

It seems to me that if `path == name`, then resetperms(path) and possibly a 
recursive call are only needed on the first call. In subsequent calls, if `path 
== name`, then we know that resetperms(path) was already called, so it 
shouldn't handle PermissionError. If resetperms was ineffective (e.g. in 
Windows, a sharing violation or custom discretionary/mandatory permissions), or 
if something else changed the permissions in the mean time, just give up 
instead of risking a RecursionError or stack overflow. For example:


@classmethod
def _rmtree(cls, name, first_call=True):
resetperms_funcs = (_os.unlink, _os.rmdir, _os.scandir, _os.open)

def resetperms(path):
try:
_os.chflags(path, 0)
except AttributeError:
pass
_os.chmod(path, 0o700)

def onerror(func, path, exc_info):
if (issubclass(exc_info[0], PermissionError) and
  func in resetperms_funcs and (first_call or path != name)):
try:
if path != name:
resetperms(_os.path.dirname(path))
resetperms(path)
try:
_os.unlink(path)
# PermissionError is raised on FreeBSD for directories
except (IsADirectoryError, PermissionError):
cls._rmtree(path, first_call=False)
except FileNotFoundError:
pass
elif issubclass(exc_info[0], FileNotFoundError):
pass
else:
raise

_shutil.rmtree(name, onerror=onerror)

--
nosy: +eryksun

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue35144] TemporaryDirectory clean-up fails with unsearchable directories

[Vidar Fauske]

Vidar Fauske  added the comment:

A somewhat easy repro:

Create the temporary directory, add a subdir (not sure if subdir truly 
necessary at this point), use `os.chdir()` to set the cwd to that subdir. Clean 
up the temp dir. The cwd should prevent the deletion because it will be "in 
use".

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue35144] TemporaryDirectory clean-up fails with unsearchable directories

[Serhiy Storchaka]

Serhiy Storchaka  added the comment:

Thank you Vidar! I wasn't sure about Windows, but was not able to reproduce 
possible failures. Your report gives a clue.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue35144] TemporaryDirectory clean-up fails with unsearchable directories

[Vidar Fauske]

Vidar Fauske  added the comment:

On Python 3.8.5 on Windows using the code from the above patch I recently got a 
stack overflow:

Thread 0x2054 (most recent call first):
  File "...\lib\concurrent\futures\thread.py", line 78 in _worker
  File "...\lib\threading.py", line 870 in run
  File "...\lib\threading.py", line 932 in _bootstrap_inner
  File "...\lib\threading.py", line 890 in _bootstrap

Thread 0x0de4 (most recent call first):
  File "...\lib\concurrent\futures\thread.py", line 78 in _worker
  File "...\lib\threading.py", line 870 in run
  File "...\lib\threading.py", line 932 in _bootstrap_inner
  File "...\lib\threading.py", line 890 in _bootstrap

Current thread 0x4700 (most recent call first):
  File "...\lib\tempfile.py", line 803 in onerror
  File "...\lib\shutil.py", line 619 in _rmtree_unsafe
  File "...\lib\shutil.py", line 737 in rmtree
  File "...\lib\tempfile.py", line 814 in _rmtree
  File "...\lib\tempfile.py", line 806 in onerror
  File "...\lib\shutil.py", line 619 in _rmtree_unsafe
  File "...\lib\shutil.py", line 737 in rmtree
  ... repeating

---


In my case, the outer `exc_info` from rmtree is:

PermissionError(13, 'The process cannot access the file because it is being 
used by another process')


And the inner exception from `_os.unlink(path)` is:

PermissionError(13, 'Access is denied')



I would say that expected behavior in this case would be to let the 'file is in 
use' error raise, instead of killing the process with an SO.

--
nosy: +vidartf

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue35144] TemporaryDirectory clean-up fails with unsearchable directories

[Serhiy Storchaka]

Serhiy Storchaka  added the comment:


New changeset e9b51c0ad81da1da11ae65840ac8b50a8521373c by Serhiy Storchaka in 
branch 'master':
bpo-26660, bpo-35144: Fix permission errors in TemporaryDirectory cleanup. 
(GH-10320)
https://github.com/python/cpython/commit/e9b51c0ad81da1da11ae65840ac8b50a8521373c


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue35144] TemporaryDirectory clean-up fails with unsearchable directories

[Serhiy Storchaka]

Change by Serhiy Storchaka :


--
keywords: +patch
pull_requests: +9622
stage:  -> patch review

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue35144] TemporaryDirectory clean-up fails with unsearchable directories

[Serhiy Storchaka]

Serhiy Storchaka  added the comment:

I am working on this. Left to test on Windows and analyze possible security 
issues.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue35144] TemporaryDirectory clean-up fails with unsearchable directories

[lilydjwg]

lilydjwg  added the comment:

Yes issue26660 is similar but not the same. On Windows it seems a read-only 
file cannot be deleted while on Linux a file resident in a non-searchable 
directory cannot be deleted.

An onerror for TemporaryDirectory will work. Also I'd like to add an optional 
dir_fd argument for onerror.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue35144] TemporaryDirectory clean-up fails with unsearchable directories

[Terry J. Reedy]

Change by Terry J. Reedy :


--
title: TemporaryDirectory can't be cleaned up if there are unsearchable 
directories -> TemporaryDirectory clean-up fails with unsearchable directories

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com