Potential thread safety issue under certain environments: Incorrectly nested style tag found #3010
Comments
abn
added
kind/bug
|
There are also other issues with the parallel installation: see #2658. |
... until the issues with 1.1.x have been resolved. See python-poetry/poetry#3010
|
I also have this problem when doing poetry (1.1.2) install in gitlab. The workaround is to disable the new installer: it seems to fail at a random point or sometimes not at all. |
|
Yeah, this should be relatively easy to fix. Just wrap a lot of calls to |
|
Yeah, I am also facing this issue on aws, when using the user-data script to install poetry packages |
|
For automation or when reliability is needed, the new installer should definitely be disabled. We have a config file in the repo disabling it by default. If anyone wants to use it (e.g. when setting up a new env), they can do so manually. |
|
I hit this couple times now, also in CI. If I keep running into this, I may disable new installer which is a bummer since it significantly speeds up the install in my project with 100+ dependencies. |
|
@Michal-Szelag-Sonos Which Python version are you using when the error occurs? |
I am seeing this on 2.7 |
|
Should happen on any Python version since |
Yes, from experience I only have seen this on Python 2.7 and not on other Python versions, I am not sure why. I don't know if it's a coincidence or not though. It may also be caused by the Note that every write in the |
|
So, silly question, can I use python3 poetry to manage a python2.7 project, assuming I would install poetry using |
Right, but I think some places are missing a lock, like this one: poetry/poetry/installation/executor.py Line 181 in bf30ca6 |
If I am not mistaken, that should work. @Michal-Szelag-Sonos It seems like you are not installing with |
|
EDIT: I made a mistake in my original report. Turns out I wasn't running with 3.8, more info down below. |
|
This could be triggered due to resource limits on the container. I wonder if this s python issue under constrained environments. @Michal-Szelag-Sonos can you provide some additional details regarding the environment, container etc? Can you create an example where we could try reproducing this under 3.8? |
|
Just hit this on a CI run on GHA on one of my projects with Pypy-3.6. Here's the full build log. |
|
@dev-zero does using poetry from master make a difference? |
|
seems ok so far: unable to trigger in 5 runs of the complete workflow. On the other hand I was not able to trigger it in a slightly different PR with version 1.1.6. |
|
I had another look at my environment and looks like I mis-spoke earlier. I upgraded one of the containers, but not the poetry one. I finished the CI upgrade environment using python3.8 and poetry 1.1.6, I also added explicit venv creation to the job since that mimics our dev environment and now I cannot reproduce this issue. I would previously hit it almost half the time, I ran it over 10 times and no failures. So, to recap, I changed 2 things:
I also tried to replicate this issue using docker on my local system, by limiting CPU and memory resources and I cannot repro this using python2.7. Our CI runs on a kubernetes cluster, backed by cloud infrastructure so I have a feeling that the behaviors on that system are different enough such that it triggers this problem. I also tried limiting IOPs on the storage driver since the issue may be related to how that subsystem behaves but I've had no luck reproducing it. This may also be related to the kernel scheduler itself which I cannot reproduce locally as well. Hard to say for sure. |
Doing poetry install causes CPU usage to reach ~100% for an instance that has only one CPU. Possible relevant issues python-poetry/poetry#3756 python-poetry/poetry#3010
|
Reviving this issue since we're seeing what looks like a locking problem or poetry is making the wrong call on the dependency install form time to time. We're using python3.8 for poetry so this isn't a 2.7 related problem. During install, poetry sometimes fails due to missing dist-info and it appears to be looking for the wrong version. Below is the output from a CI run where it fails to install The poetry config virtualenvs.create falseError log:
[tool.poetry]
name = "sample"
version = "0.0.1"
description = "Example of a failing run"
authors = ["Foo Bar <[email protected]>"]
[tool.poetry.dependencies]
python = ">=3.6,<4.0"
configparser = "^5.0.2"
pytest = "^6.2.3"
"pytest.mock" = "^3.5.1"
boto3 = "^1.17.12"
pyyaml = "^5.4.1"
[tool.poetry.dev-dependencies]
ipython = "^5.10"
[build-system]
requires = ["poetry-core>=1.0.0"]
build-backend = "poetry.core.masonry.api" |
|
@abn Any thoughts on this? ⬆️ |
|
it also fails in poetry own CI: https://github.com/python-poetry/poetry/runs/5611965628?check_suite_focus=true |
|
@michalszelagsonos the issue is likely due to the parallel execution of multiple You can workaround this by setting workers to 1. Also, try updating PS: I would recommend not disabling virtual environment creation, as sharing the system environment with poetry and other installations will only cause headaches. Besides, having a venv should reduce the probability of this collission. Further, |
|
@abn Thanks for chiming in. If I set the |
|
The installer is better in a few aspects outside of performance. Using |
|
max-workers=1 should be the default since correctness is very important |
|
@PetterS since the install itself is something that is safely repeatable and in cases where you do want that extra guarantee it might be better to explicitly control the workers, I would say that the defaults are okay for now. The reason I say this is that it is an active trade off, right now it seems that the cases where the current defaults fail are few and far between with 1.2. And the improvements that the new installer brings to the table for majority users far outweighs the correctness argument in this context. I am going to close this issue for now since we have no real action here. However, if with 1.2 people are finding the reported issue, please open a new issue and link it to this one. Friendly reminder that the old installer will be removed in a future release, use max worker configuration instead of disabling the installer. |
|
Looks like there will also be a However, neither is available with |
The original intent of this code was to workaround a thread safety issue. However, according to the `poetry` maintainer, the more "future proof" workaround is [to limit the worker count rather than disable the new installer](python-poetry/poetry#3010 (comment)). The underlying root issue is supposedly resolved in [the upcoming `1.2` release of `poetry`](python-poetry/poetry#3010 (comment)). But until then let's use the cleaner workaround.
This was added in #3459 to workaround the underlying issue of python-poetry/poetry#3010. However, `poetry` `1.2` fixed the underlying issue, and we are now on `poetry` `1.2.1` so we can remove this workaround. See also: * python-poetry/poetry#3010 (comment) * python-poetry/poetry#3010 (comment) * python-poetry/poetry#3010 (comment) * #5492
The associated ticket on Poetry (python-poetry/poetry#3010) was closed because the underlying triggers were mostly resolved in `poetry` `1.2` using the new installer. In particular, see python-poetry/poetry#3010 (comment). We flipped to the new installer in #5838, so this skip should no longer be necessary. This reverts commit ceb5700. Additionally, this flips the fixture from a Python 2 to a Python 3 fixture. We dropped support for Python 2 a while ago, but didn't notice this test needed updating because it was skipped. Another test still needs a python 2 fixture to ensure that lockfiles still pinned to python 2 throw the correct exception. So copied the existing fixture, updated it to specify a python 3 version, and then regenerated the lockfile using: ``` PYENV_VERSION=3.10.7 pyenv exec poetry lock --no-update ```
The associated ticket on Poetry (python-poetry/poetry#3010) was closed because the underlying triggers were mostly resolved in `poetry` `1.2` using the new installer. In particular, see python-poetry/poetry#3010 (comment). We flipped to the new installer in #5838, so this skip should no longer be necessary. This reverts commit ceb5700. Additionally, this flips the fixture from a Python 2 to a Python 3 fixture. We dropped support for Python 2 a while ago, but didn't notice this test needed updating because it was skipped. Another test still needs a python 2 fixture to ensure that lockfiles still pinned to python 2 throw the correct exception. So copied the existing fixture, updated it to specify a python 3 version, and then regenerated the lockfile using: ``` PYENV_VERSION=3.10.7 pyenv exec poetry lock --no-update ```
|
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. |
In certain environments, specifically CI environments, this issue occurs randomly. It would seem that when using the new executor for installation the
iobeing used is not thread safe andpastelfails attempting to pop the wrong stack.The text was updated successfully, but these errors were encountered: