From 5ab9c17367e7d0ff2f5f18bbc69fd8fc428dc17a Mon Sep 17 00:00:00 2001 From: ento Date: Fri, 24 Jul 2020 08:38:46 -0800 Subject: [PATCH 1/4] Ensure lock is released when there's an exception --- poetry/installation/executor.py | 77 ++++++++++++++------------------- 1 file changed, 33 insertions(+), 44 deletions(-) diff --git a/poetry/installation/executor.py b/poetry/installation/executor.py index c7d02df520b..b399380e9a9 100644 --- a/poetry/installation/executor.py +++ b/poetry/installation/executor.py @@ -134,32 +134,29 @@ def _write(self, operation, line): return if self._io.is_debug(): - self._lock.acquire() - section = self._sections[id(operation)] - section.write_line(line) - self._lock.release() + with self._lock: + section = self._sections[id(operation)] + section.write_line(line) return - self._lock.acquire() - section = self._sections[id(operation)] - section.output.clear() - section.write(line) - self._lock.release() + with self._lock: + section = self._sections[id(operation)] + section.output.clear() + section.write(line) def _execute_operation(self, operation): try: if self.supports_fancy_output(): if id(operation) not in self._sections: if self._should_write_operation(operation): - self._lock.acquire() - self._sections[id(operation)] = self._io.section() - self._sections[id(operation)].write_line( - " • {message}: Pending...".format( - message=self.get_operation_message(operation), - ), - ) - self._lock.release() + with self._lock: + self._sections[id(operation)] = self._io.section() + self._sections[id(operation)].write_line( + " • {message}: Pending...".format( + message=self.get_operation_message(operation), + ), + ) else: if self._should_write_operation(operation): if not operation.skipped: @@ -204,14 +201,12 @@ def _execute_operation(self, operation): self._write(operation, message) io = self._sections.get(id(operation), self._io) - self._lock.acquire() - - trace = ExceptionTrace(e) - trace.render(io) - io.write_line("") + with self._lock: + trace = ExceptionTrace(e) + trace.render(io) + io.write_line("") - self._shutdown = True - self._lock.release() + self._shutdown = True except KeyboardInterrupt: message = " • {message}: Cancelled".format( message=self.get_operation_message(operation, warning=True), @@ -221,9 +216,8 @@ def _execute_operation(self, operation): else: self._write(operation, message) - self._lock.acquire() - self._shutdown = True - self._lock.release() + with self._lock: + self._shutdown = True def _do_execute_operation(self, operation): method = operation.job_type @@ -269,14 +263,12 @@ def _do_execute_operation(self, operation): return result def _increment_operations_count(self, operation, executed): - self._lock.acquire() - if executed: - self._executed_operations += 1 - self._executed[operation.job_type] += 1 - else: - self._skipped[operation.job_type] += 1 - - self._lock.release() + with self._lock: + if executed: + self._executed_operations += 1 + self._executed[operation.job_type] += 1 + else: + self._skipped[operation.job_type] += 1 def run_pip(self, *args, **kwargs): # type: (...) -> int try: @@ -625,9 +617,8 @@ def _download_archive(self, operation, link): # type: (Operation, Link) -> Path progress.set_format(message + " %percent%%") if progress: - self._lock.acquire() - progress.start() - self._lock.release() + with self._lock: + progress.start() done = 0 archive = self._chef.get_cache_directory_for_link(link) / link.filename @@ -640,16 +631,14 @@ def _download_archive(self, operation, link): # type: (Operation, Link) -> Path done += len(chunk) if progress: - self._lock.acquire() - progress.set_progress(done) - self._lock.release() + with self._lock: + progress.set_progress(done) f.write(chunk) if progress: - self._lock.acquire() - progress.finish() - self._lock.release() + with self._lock: + progress.finish() return archive From 383780e6c6cbf423b8eb95ae3744a43432b7080e Mon Sep 17 00:00:00 2001 From: ento Date: Fri, 24 Jul 2020 09:03:54 -0800 Subject: [PATCH 2/4] Ensure the shutdown flag is set --- poetry/installation/executor.py | 55 +++++++++++++++++---------------- 1 file changed, 29 insertions(+), 26 deletions(-) diff --git a/poetry/installation/executor.py b/poetry/installation/executor.py index b399380e9a9..4fdffdeba13 100644 --- a/poetry/installation/executor.py +++ b/poetry/installation/executor.py @@ -190,34 +190,37 @@ def _execute_operation(self, operation): if result == -2: raise KeyboardInterrupt except Exception as e: - from clikit.ui.components.exception_trace import ExceptionTrace - - if not self.supports_fancy_output(): - io = self._io - else: - message = " • {message}: Failed".format( - message=self.get_operation_message(operation, error=True), - ) - self._write(operation, message) - io = self._sections.get(id(operation), self._io) - - with self._lock: - trace = ExceptionTrace(e) - trace.render(io) - io.write_line("") + try: + from clikit.ui.components.exception_trace import ExceptionTrace - self._shutdown = True + if not self.supports_fancy_output(): + io = self._io + else: + message = " • {message}: Failed".format( + message=self.get_operation_message(operation, error=True), + ) + self._write(operation, message) + io = self._sections.get(id(operation), self._io) + + with self._lock: + trace = ExceptionTrace(e) + trace.render(io) + io.write_line("") + finally: + with self._lock: + self._shutdown = True except KeyboardInterrupt: - message = " • {message}: Cancelled".format( - message=self.get_operation_message(operation, warning=True), - ) - if not self.supports_fancy_output(): - self._io.write_line(message) - else: - self._write(operation, message) - - with self._lock: - self._shutdown = True + try: + message = " • {message}: Cancelled".format( + message=self.get_operation_message(operation, warning=True), + ) + if not self.supports_fancy_output(): + self._io.write_line(message) + else: + self._write(operation, message) + finally: + with self._lock: + self._shutdown = True def _do_execute_operation(self, operation): method = operation.job_type From de597c1c8b21ecfb453f4b23cbc835e63bd371f3 Mon Sep 17 00:00:00 2001 From: ento Date: Fri, 24 Jul 2020 10:22:49 -0800 Subject: [PATCH 3/4] Add executor test for handling of io errors --- tests/installation/test_executor.py | 30 +++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/tests/installation/test_executor.py b/tests/installation/test_executor.py index a9dea0d136a..c2efab1ced1 100644 --- a/tests/installation/test_executor.py +++ b/tests/installation/test_executor.py @@ -182,6 +182,36 @@ def test_execute_should_show_operation_as_cancelled_on_subprocess_keyboard_inter assert expected == io.fetch_output() +def test_execute_should_gracefully_handle_io_error(config, mocker, io): + env = MockEnv() + executor = Executor(env, pool, config, io) + executor.verbose() + + original_write_line = executor._io.write_line + + def write_line(string, flags=None): + # Simulate UnicodeEncodeError + string.encode("ascii") + original_write_line(string, flags) + + mocker.patch.object(io, "write_line", side_effect=write_line) + + assert 1 == executor.execute([Install(Package("clikit", "0.2.3"))]) + + expected = r""" +Package operations: 1 install, 0 updates, 0 removals + + + UnicodeEncodeError + + 'ascii' codec can't encode character '\\u2022' in position \d+: ordinal not in range\(128\) + + at tests/installation/test_executor\.py:\d+ in write_line +""" + + assert re.match(expected, io.fetch_output()) + + def test_executor_should_delete_incomplete_downloads( config, io, tmp_dir, mocker, pool, mock_file_downloads ): From 00eef016ade59a5f77d1f800b3632036bc1c424e Mon Sep 17 00:00:00 2001 From: ento Date: Fri, 24 Jul 2020 11:22:10 -0800 Subject: [PATCH 4/4] Fix test for python 2.7/3.5 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. --- tests/installation/test_executor.py | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/tests/installation/test_executor.py b/tests/installation/test_executor.py index c2efab1ced1..278f4a2a138 100644 --- a/tests/installation/test_executor.py +++ b/tests/installation/test_executor.py @@ -202,11 +202,7 @@ def write_line(string, flags=None): Package operations: 1 install, 0 updates, 0 removals - UnicodeEncodeError - - 'ascii' codec can't encode character '\\u2022' in position \d+: ordinal not in range\(128\) - - at tests/installation/test_executor\.py:\d+ in write_line +\s*Unicode\w+Error """ assert re.match(expected, io.fetch_output())