Skip to content

Reachability analysis no longer idempotent? #4080

Description

@MichaReiser

The reachability analysis introduced a thread local in astral-sh/ruff#26272 to prevent stack overflows.

The issue is, that this makes Salsa queries calling this function depend on thread-local state. The query's dependencies can differ depending on which thread the query runs on. This is a real issue with cycle handling where it's not uncommon that a query first runs on thread A, but later is moved to thread B because thread B owns the cycle head. Under-tracking a query's dependencies can result in stale cache results because the query doesn't get invalidated when it should.

[ ] analyze_non_terminal_call_prefix(A)
 |
 +--[S] non_terminal_call_predicates(A)
 |
 +--[S] analyze_non_terminal_call_range(A, level, index)
     |
     +--[S] analyze_non_terminal_call_range(A, child_level, child_index)
         |
         +--[ ] analyze_non_terminal_calls(A)
             |
             +--[ ] analyze_single(call)
                 |
                 +--[S] analyze_non_terminal_call(callable, call_expr, is_await)
                     |
                     +--[ ] infer_same_file_expression_type(callable)
                         |
                         +--[ ] infer_expression_types(callable)
                             |
                             +--[S] infer_expression_types_impl(callable)
                                 |
                                 +-- ... member/place lookup and inference ...
                                     |
                                     +--[S] StaticClassLiteral::implicit_attribute_inner(...)
                                     |
                                     +--[ ] evaluate_reachability_constraint(B)
                                         |
                                         +--[ ] analyze_non_terminal_call_prefix(B)
                                             |
                                             +--[S] non_terminal_call_predicates(B)
                                             |
                                             +--[S] analyze_non_terminal_call_range(B, ...)
                                                 |
                                                 +-- ... repeats ...

See how analyze_non_terminal_call_prefix can cross query boundaries. Using thread_local is only allowed if it is fully isolated within a function and doesn't change the recorded salsa query dependencies.

Metadata

Metadata

Assignees

Labels

bugSomething isn't working

Type

No type

Projects

No projects

Relationships

None yet

Development

No branches or pull requests

Issue actions