Skip to content

fix(query): avoid cyclic package docs deadlock - #2681

Draft
Myriad-Dreamin wants to merge 26 commits into
mainfrom
fix-package-docs-type-check-deadlock
Draft

fix(query): avoid cyclic package docs deadlock#2681
Myriad-Dreamin wants to merge 26 commits into
mainfrom
fix-package-docs-type-check-deadlock

Conversation

@Myriad-Dreamin

@Myriad-Dreamin Myriad-Dreamin commented Jul 31, 2026 •
edited
Loading

Copy link
Copy Markdown
Owner
  • Maintain import SCCs with a revision-local union-find and merge late-resolved imports into fresh owners.
  • Run dependency discovery outside the graph lock and publish expression and type results only as complete component batches.
  • Share type variables across cyclic modules so back-edge constraints propagate without reading incomplete results.
  • Bound diagnostic type rendering so recursive/shared parameter types cannot exhaust SCIP generation.

@Myriad-Dreamin
Myriad-Dreamin force-pushed the fix-package-docs-type-check-deadlock branch 3 times, most recently from 47351fd to b4fa737 Compare July 31, 2026 13:38
@Myriad-Dreamin
Myriad-Dreamin force-pushed the fix-package-docs-type-check-deadlock branch from b4fa737 to bd047c0 Compare July 31, 2026 14:19
@Myriad-Dreamin
Myriad-Dreamin marked this pull request as draft August 3, 2026 09:42
Myriad-Dreamin and others added 18 commits August 6, 2026 19:31
@Myriad-Dreamin
refactor(query): split PDG data types from graph logic
c07f940
@Myriad-Dreamin
refactor(query): remove duplicate dependency evaluator
474cd96
@Myriad-Dreamin
refactor(query): move analysis tests out of module root
05a5711
@Myriad-Dreamin
refactor(query): keep analysis tests outside analysis module
dc0361c
@Myriad-Dreamin
refactor(query): colocate analysis tests with implementations
5ada787
@Myriad-Dreamin
refactor(query): colocate expression coordination tests
9510c6c
@Myriad-Dreamin
refactor(query): place component tests with pdg
8a78b12
@Myriad-Dreamin
refactor(query): keep SCC edges outside groups
06bab72
@Myriad-Dreamin
fix(query): coordinate cyclic analysis with union-find
e8ca31d
@Myriad-Dreamin @claude
refactor(query): replace union-find with relabeled groups and tombstones
c831234 
Dynamic SCC maintenance and atomic batch publication are unchanged; only
the staleness mechanism is simplified:

- A merge now eagerly relabels member files to a fresh group id and
  removes the merged groups, so a stored group id is always live and no
  find/path-compression layer is needed.
- Sealed constituents are retracted with an atomic tombstone flag in the
  same critical section that relabels their members, replacing the
  root-canonicity check; `is_current` becomes a lock-free flag read.
- The cycle region for a closing edge is computed with per-candidate
  forward searches instead of building a reverse edge index.
- The pdg tests move to pdg_tests.rs unchanged in coverage.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@Myriad-Dreamin @claude
style(query): format merge scrutinee per rustfmt
8efdf6e 
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@Myriad-Dreamin @claude
test(query): drop pdg-local tests
b56f5d1 
Cyclic coordination remains covered by the package docs tests.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@Myriad-Dreamin @claude
refactor(query): deduplicate component result plumbing
5f3b85b 
- Drop the eager component_vars pre-population; the lazy path in
  component_var creates the identical shared variable on first
  reference, and the env outlives all members of the batch.
- Extract the repeated missing-member panic into component_member.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@Myriad-Dreamin @claude
fix(query): linearize batch publication with component ownership
8dae946 
The check-then-publish sequence let a concurrent late-edge merge land
between the ownership check, the dependency-identity reads, and the
history publication. A retracted owner could publish an obsolete batch
(computed with no-wait fallbacks), and the dependency read on a merged
group answered None, panicking expr_history_key. The window predates the
tombstone refactor: the union-find version also took separate lock
acquisitions for each step.

- Add ComponentCoordinator::commit_current: verifies ownership by
  pointer identity against the live group and runs the publication in
  the same critical section as the reads, so no merge can interleave.
  Expression and type batches both publish through it.
- Add member_dependencies for one consistent snapshot on the reuse
  path; a retracted owner yields None (cache miss + retry) instead of
  a panic. expr_history_key becomes a pure function of the snapshot.
- Restore the reverse-index backward search in add_edge; the
  per-candidate forward searches degraded to O(N^2) when a long chain
  closed a cycle.
- Cover the retract/commit linearization with a coordinator test.

Nesting the cache locks under the coordinator mutex is one-directional:
no path holds an IncrCacheMap lock while acquiring the coordinator.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@Myriad-Dreamin @claude
fix(query): accept component references in ownership checks
a232e9e 
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@Myriad-Dreamin @claude
fix(analysis): make type ordering content-stable across schedules
28c6633 
Package-scan reruns produced different inferred types for identical
input. Two ordering substrates leaked scheduling into results:

- Interned hashes by pointer address, so the HashTrieSet bound sets in
  DynTypeBounds iterated in allocation order; checker invocation order
  over receiver bounds then depended on thread interleaving. The bound
  sets become RedBlackTreeSetSync, iterating in content order.
- The default Decl ordering compared raw file-id and span identities,
  which are assigned in interning order. Every sort built on it
  (iter_union, freeze, transform_let) was schedule-dependent; the
  codebase already carried the content-stable strict_cmp marked 'free
  from the concurrency issue' and a 'todo: buggy' on TypeVar::cmp.
  Decl and TypeVar now order by strict_cmp, with an equal-id fast path
  so same-file comparisons stay cheap.
- Interned<InsTy> ordering compared only values; an ordered set would
  merge instances that pointer equality keeps apart. Ties now break on
  syntax-source content (not spans, whose identities are unstable).

Known remaining suspect, deliberately deferred: BuiltinTy derives Ord
through typst's Element/Type; if those compare by static address, they
vary per process (ASLR) and will show up as residual scan diffs.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@Myriad-Dreamin @claude
fix(analysis): use TypeSource::name for the ordering tiebreak
e30055a 
SyntaxNode::into_text does not exist in this typst fork; the memoized
name accessor covers leaf and inner nodes.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@Myriad-Dreamin @claude
style(analysis): format ordering helpers per rustfmt
cb2d611 
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Myriad-Dreamin and others added 2 commits August 17, 2026 17:59
@Myriad-Dreamin @claude
fix(analysis): remove raw-identity paths from value ordering
e29c35a 
The package-scan rerun at cb2d611 cut the nondeterministic type diffs
from 193 to 175 rows but confirmed remaining flips, and pinned them on
cmp_value: functions and args compared raw span identities, modules
compared raw file ids, and color/content/symbol/styles/dyn fell back to
address comparison. All of these depend on interning or allocation
order, so sorts recursing through Interned<InsTy> stayed
schedule-dependent even after the container and Decl fixes.

- Functions order by name, then content-stable span (file path plus
  in-file span), then element name or display representation.
- Args and modules use the content-stable span/file-id comparisons.
- Types order by long name instead of typst's Ord.
- Every pointer fallback becomes a display-representation comparison;
  distinct values with an identical representation now compare equal,
  which an ordered set may collapse — acceptable, as they also render
  identically wherever a type is shown.

StrictCmp is promoted to pub(crate) for reuse; ptr_cmp is gone.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@Myriad-Dreamin @claude
style(analysis): format args ordering arm per rustfmt
ec172bb 
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Enter-tainer Enter-tainer Awaiting requested review from Enter-tainer Enter-tainer is a code owner

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@Myriad-Dreamin