Skip to content

Commit

Permalink
fix race condition and KeyError exception in executor (#9335)
Browse files Browse the repository at this point in the history
Previously, it was possible that `Executor.supports_fancy_output` would
flip-flop between True and False if a thread was updating a section.
This could lead to a crash when an operation got different reponses as
it made progress.

`Executor.supports_fancy_output` reflects the value of the underlying
cleo Formatter used by the Output object. The Formatter is shared by any
SectionOutputs derived by that Output object.

If a thread (tA) is in the middle of `Formatter.remove_format` while
updating a section, the flag to show decorator support is temporarily
toggled off and then restored. This opens a window where another thread
could get an incorrect answer when querying `supports_fancy_output`.

If a parallel thread (tB) queries `supports_fancy_output` and sees it is
False, the operation will not get added to the Executor's _sections
dictionary. If tB's operation progresses after tA has restored the
decorator value and attempts to write out progress information it will
call `Executor._write`, see that `supports_fancy_output` is now True and
attempt to find the operation in the _sections dictionary, however there
will not be an entry for that operation due to the earlier query that
returned False.

This causes tB to throw a KeyError and causes the install to shutdown.

Now, the Executor queries and caches whether the Output is decorated
during init. This value is used in `supports_fancy_output` so as to not
be affected by changes to the underlying Formatter object during section
updates.

Signed-off-by: Vincent Fazio <[email protected]>
  • Loading branch information
vfazio committed Apr 30, 2024
1 parent 6408971 commit acaf9c8
Showing 1 changed file with 5 additions and 1 deletion.
6 changes: 5 additions & 1 deletion src/poetry/installation/executor.py
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,10 @@ def __init__(
self._shutdown = False
self._hashes: dict[str, str] = {}

# Cache whether decorated output is supported.
# https://github.com/python-poetry/cleo/issues/423
self._decorated_output: bool = self._io.output.is_decorated()

@property
def installations_count(self) -> int:
return self._executed["install"]
Expand All @@ -121,7 +125,7 @@ def enabled(self) -> bool:
return self._enabled

def supports_fancy_output(self) -> bool:
return self._io.output.is_decorated() and not self._dry_run
return self._decorated_output and not self._dry_run

def disable(self) -> Executor:
self._enabled = False
Expand Down

0 comments on commit acaf9c8

Please sign in to comment.