Make parallel tox runs work with shared venvs

[obestwalter]

This would be a way of fixing #412 as @ssbarnea proposed in #412 (comment):

Mainly I would say that tox needs to get a lock on the virtualenv when is changing it. If the lock is present, tox will wait for it to be released. This should probably fix both detox and bash parallelization.

Sound reasonable to me - are there any other ideas to make this work?

[ssbarnea]

As a note: maybe this should not be linked to detox itself because it does apply to running tox in parallel even without detox. When we address it, lets try to find a solution for both cases.

[asottile]

If you're looking for cross-platform exclusive file locking, I have a bit implemented here that seems to work well on windows / linux / macos

[nicoddemus]

I've also used filelock and it worked well.

[asottile]

I wish I wrote down why I decided not to use filelock -- there was some oddity on windows iirc :/

[nicoddemus]

I wish I wrote down why I decided not to use filelock -- there was some oddity on windows iirc :/

That's good to know, thanks. I used filelock on Windows and it worked well for my simple use case. You might have hit some edge-case or bug then. 😁

[awiddersheim]

Just to note, the race condition that I point out here that deals with a shared log directory is not venv specific and has the potential to break anytime tox is running in parallel with three or more instances. The log directory is created in {workdir}/log and does not seem to be changeable. That said, detox doesn't seem to be affected since it creates a session (which does the rmtree(logdir)) only once before spawning each test. However, there are other times where tox does this directory dance. Namely with a tmp directory in each venv.

This race condition exists because of the code in py here.

Take for example this scenario:

Process[1-2] rmtree(logdir)
Process1 mkdir(logdir)
Process2 mkdir(logdir) which raises py.error.EEXIST exception
Process3 rmtree(logdir)
Process2 isdir(logdir) while logdir is missing
Process2 bubbles up a py.error.EEXIST exception

[nickwilliams-eventbrite]

Is there any compelling reason we couldn't solve this by just putting the environment name in the log directory name, so that each parallel process had its own log directory? Or, alternatively, put the environment name in the log file name(s)?

[gaborbernat]

That well could be a reasonable solution. Keeping backward compatibility, doing the change and actually testing things work is what needs to be done by someone. Such PR would have a high chance of merge.

[gaborbernat]

Also, note that if you use isolated build with tox 3.5.0 you no longer should hit this bug. Fixing it for legacy builds is what this issue is open as of today.

[gaborbernat]

toxs built in parallel mode now bypasses this issue 👍