fix(io): run blocking device calls on the io executor, not the event loop - #1199
Open
rickwierenga wants to merge 1 commit into
Open
fix(io): run blocking device calls on the io executor, not the event loop#1199rickwierenga wants to merge 1 commit into
rickwierenga wants to merge 1 commit into
Conversation
…loop FTDI's write, read and readline called into libftdi directly from async methods, blocking the event loop for the duration of the transfer. Move them onto the single worker that the io object already owns. Device open and close had the same problem in ftdi, hid and usb, and the serial port scan in serial: enumeration, configuration and the usb read buffer drain all ran on the loop. Each is now a _setup_sync that runs as one unit on the executor, so the handle is opened by the thread that later uses it and no other coroutine observes a half-configured device. Executors are created before the open and torn down when it fails, which also fixes a thread leak in serial's setup, where the "no machines found" and "multiple devices detected" paths returned without shutting theirs down. Shutdown is now non-blocking: the worker is idle by that point, so waiting on it could only stall the loop behind an unrelated queued read. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
FTDI.write,FTDI.readandFTDI.readlineareasync defbut call into libftdi directly, so every transfer blocks the event loop until it returns. TheThreadPoolExecutorthe object creates insetup()is never used for them. This moves them onto that worker.Device open and close had the same shape in
ftdi,hidandusb, and the port scan inserial:hid.setupranhid.enumerate()andhid.Device(path=…)on the loop;hid.stoprandevice.close()there.usb.setupran device discovery,set_configuration(), endpoint resolution and thewhile self._read_packet() is not Nonebuffer drain on the loop. The drain is the worst of these — an unbounded sequence of timeout-bounded blocking reads, so a device with a full buffer stalls everything.serial.setuprancomports()on the loop (a sysfs/registry sweep).Each is now a
_setup_syncthat runs as a single unit on the executor. Running it as one callable rather than wrapping the individual calls keeps the handle opened by the thread that later reads and writes it, and means no other coroutine can observe a half-configured device between the awaits —usb.read/writecheckself.devandself.read_endpoint, which were previously set several await points apart.Two fixes fall out of this:
serial.setupcreated its executor at the top and then returned from the "No machines found" and "Multiple devices detected" paths without shutting it down, leaking a thread per failed setup.wait=False, cancel_futures=True. The worker is idle by the timestop()reaches it, sowait=Truecould only ever block the loop behind an unrelated queued read.Behavior is otherwise unchanged: the same exceptions and messages, and the existing
if len(data) != 0guards around read/readline capture recording are kept, so capture files replay as before.Not included:
USB.ctrl_transferis sync and does blocking libusb I/O. Offloading it means making itasync, which is a breaking change, so it is left for a separate discussion.Testing
ruff format,ruff checkandmypyclean; full test suite passes (2090 passed, 3 skipped, 162 subtests). Not tested against hardware.🤖 Generated with Claude Code