Skip to content

DependencyGraph has unsynchronized concurrent access from background threads #1255

Description

@LalatenduMohanty

Problem

DependencyGraph has no synchronization, but background threads read it while the main thread writes to it during bootstrap.

PR #1199 added a thread pool for RESOLVE and PREPARE_SOURCE I/O. Background threads read the live graph:

# bootstrap_requirement_resolver.py:390 — background thread
self.ctx.dependency_graph.get_root_node().get_outgoing_edges(...)

The main thread concurrently mutates the same data:

# dependency_graph.py:89 — main thread
self.children.append(current_to_child_edge)

CPython's GIL happens to serialize these today, but that's an implementation accident — the language does not guarantee thread safety for mutable containers. This also violates the design intent of PR #1199, which moved methods to module-level functions specifically to avoid background threads accessing mutable state.

Fromager's default is Python 3.12; it also declares support for 3.13 and 3.14, where the free-threaded build (no GIL) makes this a real data race.

Why it hasn't caused failures yet

  1. Timing: Top-level edges are populated by _resolve_and_add_top_level() before any background work starts. Edges added later by Start.run() are non-top-level — missing one just means the resolver falls through to prev_graph or network, producing the same result.

  2. The GIL: Serializes list.append and list iteration, preventing corruption when timing does overlap. Under free-threaded Python (3.13t+), this guarantee disappears.

Affected code paths

Thread Code path Operation
Background Resolve.background_work()_bg_resolve()resolve()_resolve_from_graph() Reads root.children
Main Start.run()bt.add_to_graph()add_child() Appends to root.children
Main _handle_phase_error()remove_dependency() Mutates nodes dict, removes edges

The resolver's self._lock protects its own cache (_known_versions, _resolved_rules) but does not cover dependency_graph.

Possible fixes

  1. Add a threading.Lock to DependencyGraph — matches the pattern already used by TrackingTopologicalSorter in the same file.

  2. Snapshot top-level edges before entering the loop — pass a frozen copy to the resolver. Avoids locking since background reads only need data that's stable after setup.

  3. Have _resolve_from_graph read only prev_graph — skip the ctx.dependency_graph read entirely. The top-level info is already in the resolver's session cache by then.

Metadata

Metadata

Labels

bugSomething isn't working

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions