Skip to content

TypeSchema: terminate package dependency walk on cyclic deps - #174

Merged
ryukzak merged 4 commits into
mainfrom
fix/register-cyclic-deps
Jun 4, 2026
Merged

TypeSchema: terminate package dependency walk on cyclic deps#174
ryukzak merged 4 commits into
mainfrom
fix/register-cyclic-deps

Conversation

@ryukzak

@ryukzak ryukzak commented Jun 4, 2026

Copy link
Copy Markdown
Collaborator

Problem

mkPackageAwareResolver (in src/typeschema/register.ts) walks each package's dependency tree recursively, memoizing the built index into acc[pkgId] only after recursing into dependencies. With mutually-dependent FHIR packages — e.g. hl7.terminology.r5hl7.fhir.uv.extensions.r5 (pulled in transitively by the SQL-on-FHIR R5 closure) — the early-return guard (if (acc[pkgId]) return acc[pkgId];) never fires during the cycle, so resolution recurses forever, growing memory until the process is OOM-killed.

Fix

  • Memoize acc[pkgId] = index; before recursing into dependencies, so a cyclic dependency terminates at the guard instead of recursing forever.
  • Remove the now-redundant post-recursion assignment.
const index = mkEmptyPkgIndex(pkg);
acc[pkgId] = index; // memoize before recursing — cycle terminates at the guard
for (const resource of await manager.search({ package: pkg })) {
    ...
}

Memory consumption

Measured with /usr/bin/time -l bun test test/api/write-generator/multi-package/sql-on-fhir.test.ts (the multi-package test whose R5 closure contains the cyclic packages):

peak RSS wall time outcome
before (cyclic recursion) ~7.2 GB and climbing never finishes (killed at 150 s) runaway — OOM
after (this fix) ~1.27 GB ~1.6 s dependency walk completes

The dependency walk now terminates and the resolver is built in bounded memory. (The SQL-on-FHIR test still reports assertion failures, but on a pre-existing, unrelated issue — Base resource not found 'http://hl7.org/fhir/StructureDefinition/Library' in the org.sql-on-fhir.ig#2.1.0-pre closure — which reproduces identically on published releases and is out of scope here.)

Test

  • Adds a regression test driving registerFromManager with a mock manager whose packages form an a ↔ b cycle. A cap on manager.search() calls plus an exact "each package scanned once" assertion make a reintroduced bug fail fast and red instead of slowly exhausting memory. Verified: passes with the fix, fails immediately without it.

ryukzak added 4 commits June 4, 2026 15:46
mkPackageAwareResolver memoized the package index only after recursing
into dependencies, so mutually-dependent packages (e.g.
hl7.terminology.r5 <-> hl7.fhir.uv.extensions.r5) recursed forever until
OOM. Memoize before recursing so the cycle terminates at the guard.
Cap manager.search() calls and assert each package is scanned exactly
once, so reintroducing the bug fails red immediately instead of slowly
exhausting memory.
@ryukzak
ryukzak merged commit b4a4d39 into main Jun 4, 2026
37 checks passed
@ryukzak
ryukzak deleted the fix/register-cyclic-deps branch June 4, 2026 15:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant