diff options
| -rw-r--r-- | test/hil/helper/hil_util.py | 26 | ||||
| -rwxr-xr-x | test/hil/hil_test.py | 107 | ||||
| -rw-r--r-- | test/hil/test/stubs/hid.py | 74 | ||||
| -rw-r--r-- | test/hil/test/test_ci_select.py | 4 | ||||
| -rw-r--r-- | test/hil/test/test_hil_bounded.py | 116 |
5 files changed, 291 insertions, 36 deletions
diff --git a/test/hil/helper/hil_util.py b/test/hil/helper/hil_util.py index dfd37467a..a0279fd4c 100644 --- a/test/hil/helper/hil_util.py +++ b/test/hil/helper/hil_util.py @@ -499,9 +499,26 @@ def run_alongside(argv: list, work, timeout: int) -> subprocess.CompletedProcess return _reap() -def run_cmd(cmd: str, cwd: str | None = None, timeout: int | None = None, +def _cmd_label(cmd) -> str: + """A one-line name for a banner. An argv whose payload is a `python3 -c` program would + otherwise dump the whole body into the CI log, where run_cmd's banners are already the + noisiest thing in a failing row.""" + if isinstance(cmd, str): + return cmd + parts = [a if len(a) <= 60 else f'<{len(a)}-char program>' for a in cmd] + return ' '.join(parts) + + +def run_cmd(cmd: str | list, cwd: str | None = None, timeout: int | None = None, binary: bool = False, split_stderr: bool = False, quiet: bool = False) -> subprocess.CompletedProcess: + """Bounded subprocess: own session, killpg on expiry, rc 124 when it had to be killed. + + `cmd` is a shell STRING or an argv LIST. argv exists for a program that cannot survive + a trip through the shell -- a multi-line `python3 -c` body -- which is how the harness + runs a library call that no in-process bound can contain. A daemon thread cannot bound + a C call that holds the GIL, so for those the child process IS the bound. + """ if timeout is None: timeout = CMD_TIMEOUT # binary: raw bytes (text mode's errors='replace' mangles non-UTF-8 file content). @@ -510,7 +527,8 @@ def run_cmd(cmd: str, cwd: str | None = None, timeout: int | None = None, # still print: a killed child is always noteworthy). popen_kwargs = { 'cwd': cwd, - 'shell': True, + # a list goes straight to execve; only a string needs a shell to parse it + 'shell': isinstance(cmd, str), 'stdout': subprocess.PIPE, 'stderr': subprocess.PIPE if split_stderr else subprocess.STDOUT, } @@ -563,7 +581,7 @@ def run_cmd(cmd: str, cwd: str | None = None, timeout: int | None = None, timeout_err = _typed(err if err is not None else ex.stderr) if split_stderr and timeout_err is None: timeout_err = b'' if binary else '' - _print_banner(f'COMMAND TIMEOUT ({timeout}s): {cmd}', timeout_out, timeout_err) + _print_banner(f'COMMAND TIMEOUT ({timeout}s): {_cmd_label(cmd)}', timeout_out, timeout_err) return subprocess.CompletedProcess(args=cmd, returncode=124, stdout=timeout_out, stderr=timeout_err) except BaseException: # BaseException, not Exception (as in CPython's own subprocess.run): @@ -582,7 +600,7 @@ def run_cmd(cmd: str, cwd: str | None = None, timeout: int | None = None, raise if r.returncode != 0 and not quiet: - _print_banner(f'COMMAND FAILED: {cmd}', r.stdout, r.stderr) + _print_banner(f'COMMAND FAILED: {_cmd_label(cmd)}', r.stdout, r.stderr) elif verbose: print(cmd) print(cmd_stdout_text(r.stdout)) diff --git a/test/hil/hil_test.py b/test/hil/hil_test.py index 140860677..253fb1b98 100755 --- a/test/hil/hil_test.py +++ b/test/hil/hil_test.py @@ -320,6 +320,63 @@ LP_READER = ( ' buf += chunk\n' 'sys.stdout.buffer.write(buf)\n' ) +# Runs under hil_util.run_cmd as `python3 -c`, argv so the body needs no shell quoting. +# A PROCESS, not a thread, and not optional: cython-hidapi wraps hid_enumerate in +# `with nogil` but calls hid_open and hid_close BARE (hidapi 0.15.0 hid.pyx), so those hold +# the GIL for their whole blocking call. A daemon thread cannot bound that -- the waiter +# parks off-GIL but must reacquire the GIL to return, which the stuck thread never yields +# -- so an in-process bound is inert exactly where it is needed, and the whole worker +# freezes rather than just the call. killpg reaches a child regardless. +# +# What blocks: hidapi's hidraw backend reads `manufacturer` and `product` via udev for each +# device that reaches create_device_info_for_device, via copy_udev_string(usb_dev, +# "manufacturer"/"product") -- both usb_string_attr, served under the device lock a wedged +# usbfs ioctl holds (v6.12.96 sysfs.c:141-143). +# +# Passing BOTH ids is what keeps a wedged peer out of that path, and it does more than skip +# non-matches: hidapi only runs the cheap pre-check `if (vendor_id != 0 || product_id != 0)` +# (0.15.0 linux/hid.c:962), so an unfiltered walk sends EVERY device straight to the locked +# reads. The pre-check itself is free -- parse_hid_vid_pid_from_sysfs parses +# <sysfs_path>/device/uevent (:532) -- and both `continue`s precede +# create_device_info_for_device (:966-970 before :976). Six examples in this tree expose a +# HID interface under VID cafe, so a VID-only walk would stall on any of them wedged on a +# peer. hid_open passes the same ids through to hid_enumerate internally (:1030), so the +# filter narrows that walk too -- but a peer running THIS example still matches both ids, +# which is why the child process, not the filter, is what bounds this. +HID_ECHO = r""" +import hid, random, sys, time + +uid, budget, want_pid = sys.argv[1], float(sys.argv[2]), int(sys.argv[3], 16) +deadline = time.monotonic() + budget + +dev = None +while dev is None: + for d in hid.enumerate(0xCafe, want_pid): + if d["serial_number"] == uid: + dev = d + break + if dev is not None or time.monotonic() >= deadline: + break + time.sleep(1) +if dev is None: + sys.exit(f"HID device not found for {uid}") + +h = hid.device() +h.open(dev["vendor_id"], dev["product_id"], uid) +try: + for size in (8, 32, 63): + # Report ID (0) + payload, padded to 64 bytes + payload = bytes(random.randint(1, 255) for _ in range(size)) + h.write(bytes([0]) + payload + bytes(64 - size)) + echo = h.read(64, 2000) + if not echo or len(echo) < size: + sys.exit(f"HID echo timeout or short read ({size} bytes)") + if bytes(echo[:size]) != payload: + sys.exit(f"HID echo wrong data ({size} bytes): " + f"sent {payload.hex()} received {bytes(echo[:size]).hex()}") +finally: + h.close() +""" MTYPE_TIMEOUT = 30 # a README-sized read is <1 s; bounds a D-state hang on a wedged device @@ -358,6 +415,13 @@ def read_disk_file(uid: str, lun: int, fname: str) -> bytes: # ~5 KB of transfers plus libmtp setup takes seconds, not minutes; a larger value makes a # wedged MTP board cost that much on every retry, all charged to the pool guard. MTP_SESSION_MARGIN = 30 # transfer budget after enumeration; past it the session is killed +# room past the child's OWN enumeration budget for the echo exchange (3 x write + a 2000ms +# hidapi read) and interpreter start-up, so the outer kill only fires on a real stall +HID_ECHO_MARGIN = 30 +# hid_generic_inout's own idProduct. Pinned against the example's descriptor by +# HidEchoRunsInAChild.test_the_pid_matches_the_example, because a silent drift here would +# widen the walk back to every cafe: HID device without failing anything. +HID_INOUT_PID = 0x4012 def get_printer_dev(id: str, vendor_str, product_str, ifnum: int): @@ -1304,38 +1368,19 @@ def test_device_audio_test_freertos(board): def test_device_hid_generic_inout(board): + # The whole exchange runs in a child (see HID_ECHO): hidapi's blocking calls hold the + # GIL, so nothing in-process can bound them. run_cmd's killpg can. uid = board['uid'] - import hid # cython-hidapi (pip: hidapi, apt: python3-hid) - - timeout = enum_timeout() - dev = None - while timeout > 0: - for d in hid.enumerate(0xCafe): - if d['serial_number'] == uid: - dev = d - break - if dev: - break - time.sleep(1) - timeout -= 1 - assert dev is not None, f'HID device not found for {uid}' - - h = hid.device() - h.open(dev['vendor_id'], dev['product_id'], uid) - try: - for size in [8, 32, 63]: - # Report ID (0) + payload, padded to 64 bytes - payload = bytes([random.randint(1, 255) for _ in range(size)]) - report = bytes([0]) + payload + bytes(64 - size) - h.write(report) - echo = h.read(64, 2000) - assert echo and len(echo) >= size, ( - f'HID echo timeout or short read ({size} bytes)') - assert bytes(echo[:size]) == payload, ( - f'HID echo wrong data ({size} bytes):\n' - f' expected: {payload.hex()}\n received: {bytes(echo[:size]).hex()}') - finally: - h.close() + r = hil_util.run_cmd( + [sys.executable, '-c', HID_ECHO, uid, str(enum_timeout()), f'{HID_INOUT_PID:#06x}'], + timeout=enum_timeout() + HID_ECHO_MARGIN, split_stderr=True) + # rc 124 is run_cmd's kill: the child was still inside a hidapi call, which is the + # wedge this runs in a child FOR -- and stderr is empty there, so say so rather than + # render a bare trailing colon + detail = hil_util.cmd_stdout_text(r.stderr).strip()[:300] + assert r.returncode == 0, (f'hid_generic_inout: {detail}' if detail else + f'hid_generic_inout: the child was killed on its bound ' + f'(rc {r.returncode}) -- a hidapi call did not return') def test_device_usbtest(board): diff --git a/test/hil/test/stubs/hid.py b/test/hil/test/stubs/hid.py new file mode 100644 index 000000000..e72aeea57 --- /dev/null +++ b/test/hil/test/stubs/hid.py @@ -0,0 +1,74 @@ +# SPDX-License-Identifier: MIT +"""Scripted stand-in for cython-hidapi, for the HID_ECHO child tests. + +A real wedge cannot be manufactured on demand, so the failure modes are scripted here and +selected with FAKE_HID_MODE. Mirrors test/stubs/pymtp.py, which does the same for libmtp. +""" +import ctypes +import ctypes.util +import os +import time + +_MODE = os.environ.get('FAKE_HID_MODE', 'ok') +_UID = os.environ.get('FAKE_HID_UID', 'CAFE01') + + +def _gil_stall(): + """Block forever WITHOUT releasing the GIL -- the shape cython-hidapi's bare + hid_open()/hid_close() calls have, and the one an in-process bound cannot touch. + + PyDLL, not CDLL: CDLL releases the GIL around the call, which would make this the + easy case instead of the hard one. Resolved through find_library so a non-glibc libc + still works; PyDLL(None) is not usable here (its `sleep` returns immediately). + """ + ctypes.PyDLL(ctypes.util.find_library('c') or 'libc.so.6').sleep(3600) +_PID = int(os.environ.get('FAKE_HID_PID', '0x4012'), 16) + + +def enumerate(vid=0, pid=0): + """Real hid.enumerate(vid, pid) filters on both ids -- 0 means "any" -- and returns a + 'path' key too. The filters are applied BEFORE the locked manufacturer/product reads, + which is why passing both narrows what a wedged peer can stall.""" + if _MODE == 'wedged_enumerate': + # hidapi's hidraw backend reads `manufacturer`/`product` for every device it + # lists, both served under the device lock -- this is that stall. + while True: + time.sleep(3600) + if _MODE == 'absent': + return [] + if vid not in (0, 0xCafe) or pid not in (0, _PID): + return [] + return [{'serial_number': _UID, 'vendor_id': 0xCafe, 'product_id': _PID, + 'path': b'/dev/hidraw0'}] + + +class device: + def __init__(self): + self._last = b'' + + def open(self, vid, pid, serial): + if _MODE == 'wedged_open': + while True: + time.sleep(3600) + if _MODE == 'wedged_open_gil': + # a thread-based bound is inert against this; only killing the process works + _gil_stall() + + def write(self, report): + self._last = bytes(report) + + def read(self, size, timeout_ms): + if _MODE == 'wedged_read': + while True: + time.sleep(3600) + if _MODE == 'short_read': + return list(self._last[1:4]) + if _MODE == 'wrong_data': + return list(bytes(b ^ 0xFF for b in self._last[1:])) + return list(self._last[1:]) # the device echoes the payload, minus report ID + + def close(self): + if _MODE == 'wedged_close': + # also GIL-holding in cython-hidapi, and it runs in HID_ECHO's finally on + # every failure path + _gil_stall() diff --git a/test/hil/test/test_ci_select.py b/test/hil/test/test_ci_select.py index f3e000cb4..ace230246 100644 --- a/test/hil/test/test_ci_select.py +++ b/test/hil/test/test_ci_select.py @@ -955,7 +955,8 @@ class TestTheHarnessTestsAreNotTheHarness(unittest.TestCase): def test_the_harness_own_tests_select_nothing_on_either_axis(self): for p in ('test/hil/test/test_ci_select.py', 'test/hil/test/test_ci_metrics.py', - 'test/hil/test/test_hil_bounded.py', 'test/hil/test/stubs/pymtp.py'): + 'test/hil/test/test_hil_bounded.py', 'test/hil/test/stubs/pymtp.py', + 'test/hil/test/stubs/hid.py'): s = ci_select.classify([p], REPO, ROSTERS) self.assertFalse(s['full'], p) self.assertFalse(s['boards'], p) @@ -979,6 +980,7 @@ class TestTheHarnessTestsAreNotTheHarness(unittest.TestCase): out = subprocess.run(['git', 'ls-files', 'test/hil/test'], cwd=REPO, capture_output=True, text=True, check=True) self.assertEqual(sorted(out.stdout.split()), [ + 'test/hil/test/stubs/hid.py', 'test/hil/test/stubs/pymtp.py', 'test/hil/test/test_ci_metrics.py', 'test/hil/test/test_ci_select.py', diff --git a/test/hil/test/test_hil_bounded.py b/test/hil/test/test_hil_bounded.py index 2230ef422..9e60a19e2 100644 --- a/test/hil/test/test_hil_bounded.py +++ b/test/hil/test/test_hil_bounded.py @@ -1930,5 +1930,121 @@ class WedgedBoardCannotReportAPass(unittest.TestCase): self.assertIn('30/30', cell) +def _gil_stall_available() -> bool: + """Whether the hid stub can simulate a GIL-HOLDING stall on this host. + + It needs a libc with sleep(3) loaded through ctypes.PyDLL. Everywhere the HIL harness + actually runs that is present; where it is not, the two tests that depend on it skip + rather than fail, because their subject is the bound, not ctypes. + """ + import ctypes + import ctypes.util + try: + ctypes.PyDLL(ctypes.util.find_library('c') or 'libc.so.6') + return True + except OSError: + return False + + +class HidEchoRunsInAChild(unittest.TestCase): + """hidapi's blocking calls hold the GIL -- cython-hidapi wraps hid_enumerate in + `with nogil` but calls hid_open and hid_close bare -- so a daemon thread cannot bound + them: the waiter parks off-GIL but must reacquire the GIL to return, which the stuck + thread never yields. Only a child process can be killed regardless, which is what + run_cmd's killpg does.""" + + def _run(self, mode, uid='CAFE01', budget='0', timeout=20, pid=None): + saved = {k: os.environ.get(k) for k in ('FAKE_HID_MODE', 'FAKE_HID_UID', + 'FAKE_HID_PID', 'PYTHONPATH', + 'PYTHONSAFEPATH')} + + def restore(): + for k, v in saved.items(): + os.environ.pop(k, None) if v is None else os.environ.__setitem__(k, v) + self.addCleanup(restore) + os.environ['FAKE_HID_MODE'] = mode + os.environ['FAKE_HID_UID'] = uid + stubs = os.path.join(TEST_DIR, 'stubs') + pp = saved['PYTHONPATH'] + os.environ['PYTHONPATH'] = stubs if not pp else f'{stubs}:{pp}' + # `python3 -c` puts the cwd at sys.path[0], AHEAD of PYTHONPATH, so any hid.py + # reachable from the suite's cwd would displace the stub and every mode-driven + # test below would pass or fail for the wrong reason. Safe-path mode drops it -- + # the same practice _MtpFakeRig documents. + os.environ['PYTHONSAFEPATH'] = '1' + from helper import hil_util + want = pid or f'{hil_test.HID_INOUT_PID:#06x}' + return hil_util.run_cmd( + [sys.executable, '-c', hil_test.HID_ECHO, uid, budget, want], + timeout=timeout, split_stderr=True, quiet=True) + + def _stderr(self, r): + from helper import hil_util + return hil_util.cmd_stdout_text(r.stderr) + + def test_a_healthy_device_passes(self): + r = self._run('ok') + self.assertEqual(r.returncode, 0, self._stderr(r)) + + def test_the_pid_matches_the_example(self): + """The walk filters on BOTH ids, and hidapi applies them before the locked + manufacturer/product reads. Six examples in this tree expose a HID interface under + VID cafe, so a stale PID here silently widens the walk back to all of them -- and + nothing else would fail. Pinned against the descriptor rather than restated.""" + import re + src = (Path(TEST_DIR).parents[2] + / 'examples/device/hid_generic_inout/src/usb_descriptors.c').read_text() + m = re.search(r'#define\s+USB_PID\s+(0x[0-9a-fA-F]+)', src) + self.assertIsNotNone(m, 'hid_generic_inout no longer defines USB_PID') + self.assertEqual(hil_test.HID_INOUT_PID, int(m.group(1), 16), + 'HID_INOUT_PID drifted from the example descriptor') + + def test_a_peer_running_another_example_is_filtered_out(self): + """The point of the PID filter: a wedged sibling on a different example never + reaches the locked reads at all.""" + r = self._run('ok', pid='0x400f') # hid_composite, not ours + self.assertNotEqual(r.returncode, 0) + self.assertIn('HID device not found', self._stderr(r)) + + @unittest.skipUnless(_gil_stall_available(), 'no libc for a GIL-holding stall') + def test_a_gil_holding_stall_is_still_killed(self): + """THE case an in-process bound cannot cover. hid_open is not `with nogil`, so a + thread-based guard is inert there; the child is killed anyway.""" + t0 = time.monotonic() + r = self._run('wedged_open_gil', timeout=2) + self.assertEqual(r.returncode, 124, + 'a GIL-holding hidapi stall must still be killed on the bound') + self.assertLess(time.monotonic() - t0, 20, 'run_cmd did not bound the child') + + def test_a_wedged_enumerate_is_killed_on_the_bound(self): + r = self._run('wedged_enumerate', timeout=2) + self.assertEqual(r.returncode, 124) + + @unittest.skipUnless(_gil_stall_available(), 'no libc for a GIL-holding stall') + def test_a_wedged_close_is_killed_on_the_bound(self): + """close() runs in the child's finally on EVERY failure path and is also + GIL-holding; hidraw_release takes the same rwsem hidraw_open needs.""" + r = self._run('wedged_close', timeout=3) + self.assertEqual(r.returncode, 124) + + def test_an_absent_device_reports_why(self): + r = self._run('absent') + self.assertNotEqual(r.returncode, 0) + self.assertIn('HID device not found', self._stderr(r)) + + def test_a_bad_echo_reports_both_payloads(self): + r = self._run('wrong_data') + self.assertNotEqual(r.returncode, 0) + msg = self._stderr(r) + self.assertIn('wrong data', msg) + self.assertIn('sent', msg) + self.assertIn('received', msg) + + def test_a_short_echo_is_not_read_as_a_pass(self): + r = self._run('short_read') + self.assertNotEqual(r.returncode, 0) + self.assertIn('short read', self._stderr(r)) + + if __name__ == '__main__': unittest.main() |