-
Notifications
You must be signed in to change notification settings - Fork 2.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Race condition in new installer (1.1.0rc1) with --remove-untracked #2658
Comments
From the stack trace, I think it is plausible that running We could make sure all uninstall commands have different priorities so that they appear in different groups here: poetry/poetry/installation/executor.py Line 106 in 27d1599
|
Will investigate this more once #2624 is in. |
@PetterS is this consistently reproducible for you? I am having issues reproducing it using |
@abn not consistently as in triggering every time, no. But it really needs to be fixed before 1.1. I think the stack trace pretty clearly shows that running the installs in parallel is unsafe? |
Crashed on the second attempt with 1.1.0b2. So it should be easy to reproduce.
|
Will you accept a PR making all uninstalls run sequentially? We have to do something about this. |
@PetterS I am not sure what to tell you. I just ran this against the current master, and I did not hit the issue. Effectively sets up your issue and iterates 10 times. #!/usr/bin/env bash -e
TMP_DIR=`mktemp -d`
TOML=${TMP_DIR}/pyproject.toml
function reset-pyproject() {
cat > ${TOML} <<EOF
[tool.poetry]
name = "foobar"
version = "0.1.0"
description = ""
authors = ["Bender Rodriguez <bender@planetexpress.com>"]
[tool.poetry.dependencies]
python = "^3.8"
[tool.poetry.dev-dependencies]
ipython = "*"
EOF
}
mkdir -p ${TMP_DIR}/foobar
touch ${TMP_DIR}/foobar/__init__.py
pushd ${TMP_DIR}
POETRY=poetry
for i in $(seq 1 10); do
reset-pyproject
${POETRY} lock
${POETRY} install
sed -i /ipython/d ${TOML}
${POETRY} lock
${POETRY} install --remove-untracked
done
popd I am curious as to what the root cause here is, considering each package is only uninstalled once, how two packages try to remove the same file/directory is confounding. @sdispater you have any input here since you implemented the new installer? Would also be great if you can see if you can reproduce the issue. |
Any progress here? This still looks like a concurrency bug to me. |
Your script does not reproduce the issue on my desktop computer (16 cores/32 threads Ubuntu 20.04 WSL2). Your script does reproduce the issue on my laptop (Ubuntu 20.04 WSL1). First attempt as well. Both with Poetry 1.1 RC1. |
It is a pretty easy change to make all uninstalls run sequentially. Will not affect performance that much either. |
@PetterS as I mentioned before I am a bit hesitant to recommend the change until we know the root cause of the issue. As in why two operations are attempting to remove the same file as you have indicated. |
The crashing code is this:
The last line fails with a Does pip even document that concurrent uninstalls are safe? |
Here is the smoking gun, I think! 🥳
The leading "~" can be seen in the stack trace. |
@abn @sdispater I can make a PR, but need to know that it will be accepted. It is an easy fix. |
This is also causing issues with source packages. I'm installing source packages from our self-hosted git repo, and there is a clear race condition when using the new installer. I originally blamed #2102, but after some experimentation determined that the issue was unrelated to that. My symptoms are that some of my source packages do not exist in my environment after doing the install. The happens both for system installs and venvs. Digging in, I find that with Further digging shows that the paths come from I suspect that there is a race condition on the installation of these: multiple separate processes/threads modify the file at the same time without taking a lock. There's a few options I can see to fix this, though I'm sure this is a non-exhaustive list:
I worked around this problem by disabling the experimental installer. Downgrading to 1.0.10 also did the trick. Neither of these are fantastic solutions. I suppose I could update all the dependencies to use poetry, but that may not always be possible, and it adds quite a bit of work in the short term to unblock my builds. |
@klarose this was, funnily enough, brought up on discord a few hours ago via #3086. I'd like to treat the issue being discussed here slightly separate to the issue you have raised as they are different enough to warrant separate solutions. The |
This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
To reproduce:
ipython = "*"
as a dev dependency to a repo.poetry lock
poetry install
poetry lock
poerty install --remove-untracked
I get this:
I don't know what
site-packages/~typrocess-0.6.0.dist-info'
is but my guess is that it has something to do with the parallel processing. Runing the command again fixes the issue.I suggest that we do not process uninstalls in parallel unless this is easy to fix. I don't know how to fix this. @sdispater wrote the new installer.
The text was updated successfully, but these errors were encountered: