Skip to content

fix(utilities): serialise first resolution of lazy exports and module descriptors so concurrent imports cannot deadlock (#2771) - #2781

Merged
JarryShaw merged 2 commits into
mainfrom
fix/2771-lazy-export-deadlock
Nov 4, 2026
Merged

JarryShaw merged 2 commits into
mainfrom
fix/2771-lazy-export-deadlock

Conversation

@JarryShaw

@JarryShaw JarryShaw commented Nov 4, 2026 •

Copy link
Copy Markdown
Owner

What is the purpose of your pull request?

  • fix — corrects a defect

Description of your pull request and other information

Since #2757, threads that reach different lazy names or first dispatches at the same time, on a cold import, deadlock the import system (_DeadlockError). A family member's module imports its base package's modules while another thread holds that package's import lock.

  • Shared lock: pcapkit.utilities.importlib.import_lock, a re-entrant lock, now serialises the first resolution in lazy_exports. A name, once resolved, is bound on its module, so later reads take no lock.
  • Descriptors: ModuleDescriptor.klass takes the same lock, but only when its module isn't imported yet. This covers next-layer dispatch and get_protocol.
  • IGMP: its base and member imports now go through descriptors too.
  • Registry lock: ProtocolBase._registry_lock is removed, as ruled on refactor(protocols): align ProtocolBase.__registry__ handling and entry layout with the other registry maps #2773 (option c).
  • Speed: import time is unchanged (572 vs 571 ms). Warm .klass is unchanged, and warm get_protocol drops from 460 to 348 ns.
  • Tests: four cold-process race tests cover lazy names, nesting, dispatch, and get_protocol raced with dispatch. On main they fail in 19–20 of 20 trials on 3.14; with this fix, 0 of 100. A check also fails on any first import made without the lock.
  • Follow-up commit: the remaining function-level first imports (HTTPv1/2, LinuxSLL1/2, L2TPv2, the port-registry __index__ hooks, the ip_pseudo helpers, the engines' toolkit) now go through module-level descriptors. A new static test scans every function body and fails on an unlisted first import of a module import pcapkit leaves unloaded. Each remaining plain import is listed with its reason.

Closes #2771
Part of #2773

… descriptors so concurrent imports cannot deadlock (#2771)

- Threads making first imports at once raised _DeadlockError: a family member's
  protocol module imports a module of its base's package, and the import system
  takes that module's lock before waiting on the base's package, while a thread
  importing the package takes the two the other way.
- pcapkit.utilities.importlib.import_lock, a shared RLock, is held by every first
  resolution in lazy_exports and by ModuleDescriptor.klass when its module is not
  imported yet; resolved names and imported modules are read without it.
- IGMP's base and version classes on the dispatch path resolve through descriptors
  rather than function-level imports, which bypassed the lock.
- ProtocolBase._registry_lock goes (#2773): get_protocol resolves through the lock.
- The ALL-tier race test covers lazy names, family-member dispatch and get_protocol,
  and fails on any first import made without the lock; it fails on the unfixed tree.
@JarryShaw JarryShaw added fix Pull requests that fix a defect (fix: subject prefix) review: running A cross-review is in flight against the current head - no verdict yet labels Nov 4, 2026
@github-actions github-actions Bot added this to the PyPCAPKit v1.6 milestone Nov 4, 2026
@JarryShaw JarryShaw moved this to In review in PyPCAPKit Nov 4, 2026
… the shared import lock (#2771)

- Function-level imports of modules `import pcapkit` leaves unloaded made their first
  import without import_lock, so they could still deadlock against a locked one.
- They resolve through module-level ModuleDescriptor constants instead, as IGMP's do:
  HTTP's and LinuxSLL's members and data models, L2TP's member, the TCP and UDP port
  registries of the __index__ hooks, the ip_pseudo helpers of IPv4, IPv6, ICMPv6, TCP,
  UDP and DCCP, verify_upper's AH, and the PCAP and PCAP-NG engines' toolkits.
- The race test makes these first resolutions at once with checksums verified, and a
  new project test fails on any function body importing a module first, bar the
  sites it allows with their reasons.
@github-actions

github-actions Bot commented Nov 4, 2026 •

Copy link
Copy Markdown
Contributor

Coverage: 94.45% (unit tier, Python 3.15, 89626e4d9, Unit Tests run success)

Package Statements Missed Branches Partial Cover
pcapkit (top level) 106 4 20 2 95.24%
pcapkit/const 20918 175 2908 156 98.57%
pcapkit/corekit 2918 77 942 38 96.81%
pcapkit/dumpkit 286 3 102 3 98.45%
pcapkit/foundation 3510 27 1216 32 98.75%
pcapkit/interface 138 4 52 3 96.32%
pcapkit/protocols 22839 157 5950 127 99.00%
pcapkit/toolkit 961 26 342 8 96.62%
pcapkit/utilities 565 7 152 5 98.33%
pcapkit/vendor 5590 2286 1280 214 55.75%

Per-file detail: the coverage-html artifact of this run.

@JarryShaw

Copy link
Copy Markdown
Owner Author

GOOD TO GO at 89626e4d9. Two Sonnet cross-reviews, one per commit, plus my own run of the race tests.

  • Deadlock fixed: lazy-name, dispatch and get_protocol races deadlock in 20 of 20 cold trials on main and 0 of 20 here.
    • No hang in 75 two-thread runs, nor in the nested or descriptor-vs-lazy probes.
    • import pcapkit time is unchanged, and warm get_protocol drops from about 470 to 350 ns.
  • API:
  • Follow-up commit: the rewritten sites resolve to the same objects as before. That covers HTTPv1/v2 selection, the port __index__ values (TLS 443, FTP 21, HTTP 80, DTLS 5684, FTP_DATA 20), and checksum verification with CHECKSUM_VERIFY on. The new static test fails when one site goes back to a direct import.
  • CI: Project tests, Lint, deploy-pages and every Engines cell pass.
  • Residual, unchanged from main: a user's own direct import_module racing a lazy resolution can still raise a detected _DeadlockError. It never hangs.

@JarryShaw JarryShaw added review: good-to-go Cross-review at the current head says ready; CI state is separate and removed review: running A cross-review is in flight against the current head - no verdict yet labels Nov 4, 2026
@JarryShaw
JarryShaw merged commit 812ed0f into main Nov 4, 2026
66 checks passed
@JarryShaw
JarryShaw deleted the fix/2771-lazy-export-deadlock branch November 4, 2026 21:19
@github-project-automation github-project-automation Bot moved this from In review to Done in PyPCAPKit Nov 4, 2026
@JarryShaw JarryShaw removed the review: good-to-go Cross-review at the current head says ready; CI state is separate label Nov 4, 2026
JarryShaw added a commit that referenced this pull request Nov 4, 2026
- #2777: the cut-header sentence reads the length back too, the rebuild read
  at the width its parse was given (__short_width__); the ruling into the
  conventions entry.
- #2778: a PCAP-NG block with octets after opt_endofopt rebuilds an info
  holding its options and padding_opts, where 1.5.1 rebuilt none, into the
  whole-header rebuild entry; 1.5.0's frozen entry is left as written.
- #2779: SCTP's reorder, the source-ordered member lists and the sorted
  Option.register clash warning ('opt', 'if', 'epb', ... in 1.5.1), into the
  member-order entry; the layer-grouped registry and the warning name restore
  ('put back silently' in 1.5.1), into the #2770 entry; the page wording into
  the conventions entry.
- #2781: import_lock serialising first resolutions, into the lazy-import
  entry; the removed registry lock never shipped and stays unnamed.
- CHANGELOG.md regenerated; util/changelog_md.py --check passes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

fix Pull requests that fix a defect (fix: subject prefix)

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

fix(utilities): concurrent first access to lazy protocol exports deadlocks the import system

1 participant