Skip to content
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

Fix executor deadlock when IO doesn't support non-ascii characters #2721

merged 4 commits into from Aug 25, 2020


Copy link

@ento ento commented Jul 24, 2020

My environment happened to have a faulty locale setup and running poetry install got stuck for no apparent reason. This PR makes it so that the command doesn't get stuck in similar situations.

How to repro against 1.1.0b2

# set up a new project
mkdir test && cd test
python3.8 -m venv env
. env/bin/activate
pip install -U pip
pip install
poetry init -n

# use a clean virtualenv
poetry env use python

# note: the filetype package is just an example
# it's the first package I found that doesn't have external dependencies
# (to keep dependency resolution quick)
PYTHONIOENCODING=ascii poetry add filetype

Expected: fails with a nice error

Actual: gets stuck waiting for a threading.Lock to become available.

Using version ^1.0.7 for filetype

Updating dependencies
Resolving dependencies... (0.1s)

Writing lock file

Package operations: 1 install, 0 updates, 0 removal
# no progress beyond this


sudo $(which strace) -p $(ps -C poetry -o pid=)

Ctrl-C gets it unstuck, but the new package isn't installed (while it's added to pyproject.toml).

With this fix

# clean up
$ poetry remove filetype

$ pip install path/to/poetry-checkout
$ PYTHONIOENCODING=ascii poetry add filetype                                                           
Using version ^1.0.7 for filetype                                                                      

Updating dependencies
Resolving dependencies... (0.1s)

Package operations: 1 install, 0 updates, 0 removals

Failed to add packages, reverting the pyproject.toml file to its original content.

Note: the expected output in the new test added by this PR includes information about the exception (the "Unicode\w+Error" part) because the io object used in tests doesn't support ansi.

On the other hand, when the io object supports ansi and PYTHONIOENCODING=ascii is set like in the case above, the exception handling code tries to write a message that contains a unicode character that cannot be encoded with ascii (), resulting in no information about the exception getting logged. PYTHONIOENCODING=ascii poetry add filetype | tee /dev/null is one way to see the exception.

A more thorough 'fix' for environments that don't support non-ascii characters might be to add a check for it somewhere earlier in the app, like click does. I'm leaving such changes out of this PR to keep it focused on fixing the deadlock and also because I'm not sure what the desired behavior would be (the app could exit with an error message, or it could switch to a non-ascii output format).

Pull Request Check List

Resolves: no corresponding issue

  • Added tests for changed code. (sorta; not all changes might not be covered by new and existing tests)
  • Updated documentation for changed code.

ento added 4 commits Jul 24, 2020
The exception class is UnicodeDecodeError in these versions
and also the preceding whitespace doesn't seem to be there.

This change makes the regex pattern more accommodating while
still asserting that there was an error related to unicode.
@finswimmer finswimmer requested a review from a team Aug 3, 2020
abn approved these changes Aug 25, 2020
@abn abn merged commit a3ef6dc into python-poetry:master Aug 25, 2020
@abn abn added the kind/bug Something isn't working as expected label Aug 25, 2020
@abn abn added this to In progress in 1.1 via automation Aug 25, 2020
@ento ento deleted the fix-executor-deadlock branch Aug 25, 2020
@sdispater sdispater mentioned this pull request Sep 18, 2020
@abn abn moved this from In progress to Done in 1.1 Sep 23, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
kind/bug Something isn't working as expected
No open projects

Successfully merging this pull request may close these issues.

None yet

2 participants