tmpdir_factory deletes tmp dirs from other pytest processes after 3 hours, even if they are still running

[danc86]

We use pytest in a Jenkins job, which ends running many concurrent pytest processes on a single Linux worker under the same user account (jenkins). The test suite has some session-scoped fixtures which use tmpdir_factory to create temporary directories and create files in them, which are then used throughout the duration of the entire test run.

Recently our pytest runs started taking longer than 3 hours and we started to see tests randomly fail near the end of the suite with errors like this:

FileNotFoundError: [Errno 2] No such file or directory: '/tmp/pytest-of-jenkins/pytest-47927/...

indicating that something had deleted the tmp files created by the session-scoped fixture at the beginning of the test run.

Pytest's tmpdir_factory fixture calls make_numbered_dir_with_cleanup to handle creation and cleanup of tmp directories:
https://github.com/pytest-dev/pytest/blob/4.4.0/src/_pytest/tmpdir.py#L75
That function has some logic which looks for the presence of a lock file to detect if the directory is still being used by another pytest process:
https://github.com/pytest-dev/pytest/blob/4.4.0/src/_pytest/pathlib.py#L205
but the logic assumes that any lock file older than the given "lock timeout" (hardcoded to 3 hours) is stale and can just be deleted. But if the other pytest process is actually still running after 3 hours, its tmp directory will be deleted while it is still in use.

The Linux worker is CentOS 7.8 with Python 3.6.8 and pytest 4.4.0. I see the pytest master branch still has the same 3 hour timeout logic so I think this bug will affect all recent versions of pytest.

[danc86]

Rather than checking modtimes, it would be nicer to use an actual advisory lock (as in flock). If the lock is still held by another process then you know it's alive, regardless how old the lock file is. If it's not held, then the other process died and the directory is stale and can be cleaned up. No need to pick a hardcoded "stale" timeout.

I could post a PR for that. However I am not sure how to make that portable to non-UNIX platforms. I don't use Windows.

[RonnyPfannschmidt]

I recall the timeout being much larger (either 24h or 3d)

Threading this as regression for now

[danc86]

It's hardcoded as 60 * 60 * 3 seconds = 3 hours on master (and has been unchanged for quite a few versions, according to the git history):

https://github.com/pytest-dev/pytest/blob/master/src/_pytest/pathlib.py#L35

[Zac-HD]

Using flock can cause trouble if test sessions die without releasing the lock via something like an interpreter segfault.

Personally I'd be happy to just increase the timeout to three days, as a simple solution. (we could also make LOCK_TIMEOUT a default argument value instead of a global, since it's only ever used in tmpdir)

[RonnyPfannschmidt]

d76fa59 introduced it,

i believe it is a argument and the global is the default
pylib originally had https://github.com/pytest-dev/py/blob/d4111cae5572809a23d3d5cc88bed8e4af720929/py/_path/local.py#L840-L841 - so i believe we should add it again

[nicoddemus]

Personally I'd be happy to just increase the timeout to three days, as a simple solution.

I agree increasing to a few days is simpler and probably good enough.

[danc86]

A flock is associated with an open file descriptor so it is always released whenever the process holding the flock exits (whether cleanly or abnormally, due to a signal, etc), the OS guarantees this. It can't be accidentally leaked. So flock would be the right mechanism to use for this, on UNIX at least.

[danc86]

@vgerak are you planning to file a PR to increase the timeout to 3 days? Or would you like me to?