Skip to content

Commit

Permalink
device.hardware: interrupt USB polling thread before threading shutdown.
Browse files Browse the repository at this point in the history
While performing `USBContext.handleEvents()` the poller thread is
uninterruptible other than through libusb1 API. Since the Python runtime
joins all non-daemon threads on threading shutdown it is necessary to
interrupt the thread by calling the corresponding libusb1 function.

A seemingly obvious solution is to make the thread a daemon thread so
that it is not joined on threading shutdown. Unfortunately this only
makes the problem worse. All daemon threads are instantly killed on
shutdown, leaving a lock within libusb1 permanently taken, and causing
a hang later when python-libusb1 objects are garbage collected by
the main thread.

Fixes #413.
  • Loading branch information
whitequark committed Oct 29, 2023
1 parent 5a9b10f commit 23f815f
Show file tree
Hide file tree
Showing 3 changed files with 24 additions and 10 deletions.
16 changes: 15 additions & 1 deletion software/glasgow/device/hardware.py
Original file line number Diff line number Diff line change
Expand Up @@ -54,9 +54,23 @@ def __init__(self, context):
self.context = context

def run(self):
# The poller thread spends most of its life in blocking `handleEvents()` calls and this can
# cause issues during interpreter shutdown. If it were a daemon thread (it isn't) then it
# would be instantly killed on interpreter shutdown and any locks it took could block some
# libusb1 objects being destroyed as their Python counterparts are garbage collected. Since
# it is not a daemon thread, `threading._shutdown()` (that is called by e.g. `exit()`) will
# join it, and so it must terminate during threading shutdown. Note that `atexit.register`
# is not enough as those callbacks are called after threading shutdown, and past the point
# where a deadlock would happen.
threading._register_atexit(self.stop)
while not self.done:
self.context.handleEvents()

def stop(self):
self.done = True
self.context.interruptEventHandler()
self.join()


class GlasgowHardwareDevice:
@classmethod
Expand Down Expand Up @@ -228,8 +242,8 @@ def modified_design(self):
return self._modified_design

def close(self):
self.usb_poller.done = True
self.usb_handle.close()
self.usb_poller.stop()
self.usb_context.close()

async def _do_transfer(self, is_read, setup):
Expand Down
12 changes: 6 additions & 6 deletions software/pdm.min.lock
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
groups = ["default", "builtin-toolchain", "http"]
strategy = ["cross_platform", "direct_minimal_versions"]
lock_version = "4.4"
content_hash = "sha256:8f3a6d83a8f959b68ed91a06346c34d47403afa5cb1c6f926db996addce63bb7"
content_hash = "sha256:d8664d854e834393abda2400adb7cc1f1cdd037e7c4c22b497b237756f31243f"

[[package]]
name = "aiohttp"
Expand Down Expand Up @@ -269,13 +269,13 @@ files = [

[[package]]
name = "libusb1"
version = "2.0.1"
version = "3.1.0"
summary = "Pure-python wrapper for libusb-1.0"
files = [
{file = "libusb1-2.0.1-py3-none-any.whl", hash = "sha256:81381ce1d8852a4d4345b2ee8218971d35865b5f025fef96b43ee082757099cd"},
{file = "libusb1-2.0.1-py3-none-win32.whl", hash = "sha256:9fda3055c98ab043cfb3beac93ef1de2900ad11d949c694f58bdf414ce2bd03c"},
{file = "libusb1-2.0.1-py3-none-win_amd64.whl", hash = "sha256:a97bcb90f589d863c5e971b013c8cf7e1915680a951e66c4222a2c5bb64b7153"},
{file = "libusb1-2.0.1.tar.gz", hash = "sha256:d3ba82ecf7ab6a48d21dac6697e26504670cc3522b8e5941bd28fb56cf3f6c46"},
{file = "libusb1-3.1.0-py3-none-any.whl", hash = "sha256:9d9f16e2c199cab91f48ead585d3f5ec7e8e4be428a25ddfed22abf786fa9b3a"},
{file = "libusb1-3.1.0-py3-none-win32.whl", hash = "sha256:bc7874302565721f443a27d8182fcc7152e5b560523f12f1377b130f473e4a0c"},
{file = "libusb1-3.1.0-py3-none-win_amd64.whl", hash = "sha256:77a06ecfb3d002d7c2ce369e28d0138b292fe8db8a3d102b73fda231a716dd35"},
{file = "libusb1-3.1.0.tar.gz", hash = "sha256:4ee9b0a55f8bd0b3ea7017ae919a6c1f439af742c4a4b04543c5fd7af89b828c"},
]

[[package]]
Expand Down
6 changes: 3 additions & 3 deletions software/pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -46,21 +46,21 @@ dependencies = [
# `libusb1` is used to communicate with the device, and its API mirrors the stable API/ABI of
# of the native `libusb1` library. It increases major version when dropping support for older
# Python versions.
"libusb1>=2.0.1",
"libusb1>=3.1.0",
# `pyvcd` is used in the applet analyzer to dump waveform files. It is also a dependency of
# Amaranth, and the version range here must be compatible with Amaranth's.
"pyvcd>=0.2,<0.5",
]

[project.optional-dependencies]
# By default, Glasgow uses the YoWASP (https://yowasp.org/) to build bitstreams. YoWASP is not
# installable on certain architectures and platforms, so this dependency is optional
# installable on certain architectures and platforms, so this dependency is optional.
builtin-toolchain = [
# The version of `amaranth-yosys` is constrained by Amaranth; this optional dependency only
# includes it in the dependency resolution set.
"amaranth-yosys",
# `yowasp-runtime` is a dependency of other toolchain components, and it is constrained here to
# the lowest that has features required for isolated builds.
# the lowest that has features transitively required by Glasgow.
"yowasp-runtime>=1.42",
# Most versions of Yosys and nextpnr work; the minimum versions listed here are known good.
"yowasp-yosys>=0.31.0.13",
Expand Down

0 comments on commit 23f815f

Please sign in to comment.