internal/imodels: package-level *Package cache leaks across scans, drives OOM in long-running embeddings
- Dominant language
- Go
- Stars
- 11k
- Forks
- 792
- Avg merge
- 1d 20h
- Merged PRs (30d)
- 48
Description
## Summary
`internal/imodels/imodels.go` declares a package-level cache:
```go
// todo: SBOM special case, to be removed after PURL to ESI conversion within each extractor is complete
var cache = sync.Map{} // map[*extractor.Package]*models.PackageInfo
```
`toCachedPackageInfo` writes into it for every SBOM-source package, keyed by `*extractor.Package`. Because the pointer is allocated fresh per `DoScan` (extractors produce new `Package` instances each run), entries are **never reused across scans**. The map only
grows, and because `sync.Map` retains its keys, each entry transitively pins the entire `*Package` subgraph — including the parsed JSON object allocations made during CDX/SBOM extraction — for the lifetime of the process.
For one-shot CLI use this is invisible. For long-running embeddings, it drives heap growth until OOMKill.
## Evidence
Reproduced in production via Datadog Continuous Profiler on a worker calling `osvscanner.DoScan` repeatedly from a long-lived process. Heap-live-size comparison across an 18-hour window on a single pod (no restart):
| Window | Time | Heap live size |
| --- | --- | --- |
| A | T₀ | 180 MiB |
| B | T₀ + 18h | **1.46 GiB** (+1.29 GiB) |
Dominant new retention path in the diff:
```
DoScan → … → Ecosystem (imodels.go:125)
→ toCachedPackageInfo
→ sync.Map.Store
→ HashTrie nodes ← cache structure
```
- The matcher / `ZipDB` chain shows up in **A** but **not B**, confirming the matcher is correctly released per scan (per the comment at `localmatcher.go:53-55`). The retention is exclusively the `imodels.cache`.
- The bloated `runExtractor → Extract → enumerateComponents → decodeState.*` allocations seen as live heap in window B are reachable transitively through the cache keys — fixing the cache releases that whole subgraph.
I confirmed the same code is still present on `main` (not fixed since `v2.3.5`).
## Root cause
The cache only adds value within a single `DoScan` call: `Name`, `Ecosystem`, and `Version` each call `toCachedPackageInfo(pkg)` on the same `*Package`, so the cache deduplicates those 3 lookups per package. **Across scans the keys are fresh pointers, so there
is never a cross-scan cache hit.**
Net effect:
- **Benefit:** saves 2 redundant `converter.ToPURL` + `purl.ToPackage` round-trips per package per scan (sub-millisecond total per scan).
- **Cost:** unbounded heap growth proportional to total packages ever scanned by the process.
## Possible fixes
Ordered by invasiveness:
### 1. Per-scan cache (cleanest)
Plumb the cache through `ScannerActions` / scan context so it lives only for the duration of one `DoScan`. Preserves the within-scan dedup benefit, drops the cross-scan leak. Touches the call sites of `Name` / `Ecosystem` / `Version` so they receive the cache.
### 2. Drop the cache entirely
Recompute the PURL on each call. Sub-millisecond cost per scan, no concurrency concerns, smallest diff. The upstream `todo:` comment already suggests this code is interim, so dropping the cache half of it aligns with the stated direction (the PURL → ESI
conversion plan removes the need for it altogether).
### 3. Reset hook + `DoScan` defer (not recommended)
Not safe in the simple form:
- `cache = sync.Map{}` is a struct-copy data race against concurrent `cache.Load`/`Store` calls from in-flight scans (the embedding case has multiple concurrent scans).
- `cache.Range` + `cache.Delete` avoids the race but lets one scan wipe another's in-flight entries.
- `atomic.Pointer[sync.Map]` swap fixes both but adds machinery for a cache that delivers no measurable benefit.
I'd recommend **(1)** or **(2)** over **(3)**. Happy to send a PR for whichever direction you prefer.
Contributor guide
Assessment
This issue has not been assessed yet.