Skip to content

Python: fix AttributeError in TyTypesClient.shutdown - #8434

Merged
knutwannheden merged 1 commit into
mainfrom
python-ty-client-shutdown-breaker
Aug 10, 2026
Merged

Python: fix AttributeError in TyTypesClient.shutdown#8434
knutwannheden merged 1 commit into
mainfrom
python-ty-client-shutdown-breaker

Conversation

@knutwannheden

@knutwannheden knutwannheden commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

Motivation

TyTypesClient.shutdown sends a bounded "shutdown" request and then reaps the process. That request counts toward the consecutive-timeout circuit breaker introduced in #8423, so when it happens to be the third consecutive timeout — the realistic case being a ty process that has already wedged at the end of a parse — _kill sets self._process to None before shutdown reaches self._process.wait(timeout=5). The resulting AttributeError: 'NoneType' object has no attribute 'wait' is not covered by the surrounding except (subprocess.TimeoutExpired, OSError), so it propagates out of the finally block in handle_parse_project and fails the entire ParseProject RPC — discarding every source file that had already been parsed successfully, instead of degrading gracefully to untyped LSTs.

Two smaller items from the same change ride along: __init__ initialized _responses and _consecutive_timeouts only for _start_process to overwrite both before any reader thread exists, and several comments needed a discipline pass.

Summary

  • shutdown holds the process in a local before sending the request, so reaping is unaffected by the breaker detaching self._process. This also removes the now-redundant is not None guard in the except branch.
  • Dropped the dead _responses / _consecutive_timeouts assignments from __init__. _start_process is the load-bearing one: it must reset both on a restart so a stale queue or timeout count cannot leak across ty sessions.
  • Comment pass over the lines Python: fix ty-types client deadlock and restore type attribution on Windows #8423 touched: two comments in ty_client.py and one in the test module were over the configured 120-column line-length, and the _start_process comment referred to select(), which the file no longer uses. Two comments that restated adjacent code were dropped, and an unused sys import removed from the test module.

Test plan

  • New test_shutdown_when_its_own_request_trips_the_breaker fails on main with the exact AttributeError at ty_client.py:350 and passes here. It drives a new WEDGED_SERVER fake that answers initialize and then goes silent, leaving _consecutive_timeouts at 2 so that the shutdown request itself trips the breaker.
  • pytest tests/python/test_ty_client.py — 5 passed.
  • pytest tests/python — 1603 passed, 10 skipped.
  • ruff check --select E501,F401 clean on both changed files.

`shutdown` sends a bounded "shutdown" request and then reaps the process.
That request counts toward the consecutive-timeout breaker added in #8423,
so when it is the third consecutive timeout, `_kill` sets `_process` to
None before `shutdown` reaches `self._process.wait(...)`. The resulting
AttributeError is not covered by the surrounding
`except (subprocess.TimeoutExpired, OSError)`, so it escapes the `finally`
block in `handle_parse_project` and fails the whole ParseProject request,
discarding every file already parsed.

Also drops the `__init__` copies of `_responses` and `_consecutive_timeouts`
that `_start_process` immediately overwrites, and brings the comments and
imports added in #8423 in line with the project's line length.
@knutwannheden
knutwannheden force-pushed the python-ty-client-shutdown-breaker branch from baed0eb to 8d6ed15 Compare August 10, 2026 07:08
@knutwannheden
knutwannheden merged commit 1a7c2d5 into main Aug 10, 2026
1 check passed
@knutwannheden
knutwannheden deleted the python-ty-client-shutdown-breaker branch August 10, 2026 07:39
@github-project-automation github-project-automation Bot moved this from In Progress to Done in OpenRewrite Aug 10, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

1 participant