torch.compile: isinstance() with a runtime_checkable Protocol constant-folds the result without guarding the attributes it reads
- Dominant language
- Python
- Stars
- 103k
- Forks
- 29.5k
- PR merge metrics
- PR metrics pending
Description
Found while working on #195969. The union/tuple bug reported there is a separate
issue; this one is about the single-class path that already works "correctly" on
main, and it predates that report. Analysis and reproducer below were produced
with an AI assistant and are quoted as such; I verified the reproducer myself on
current main.
> ### Summary
>
> `isinstance(obj, SomeProtocol)` is constant-folded into the graph at trace time,
> but no guard is installed on the attributes the Protocol check actually reads.
> A `@runtime_checkable` Protocol with non-method (data) members inspects the
> **instance**, while the cache entry only discriminates on the **type**. Two
> instances of the same class that differ in whether the attribute is set share a
> cache entry, so the second one silently gets the first one's answer.
>
> ### Reproducer
>
> ```python
> from typing import Protocol, runtime_checkable
>
> import torch
>
>
> @runtime_checkable
> class HasPorts(Protocol):
> ports: tuple[int, ...]
>
>
> class Obj:
> def __init__(self, with_ports):
> if with_ports:
> self.ports = (1, 2)
>
>
> def fn(x, o):
> return x + 1 if isinstance(o, HasPorts) else x - 1
>
>
> x = torch.ones(3)
> opt_fn = torch.compile(fn, backend="eager")
>
> has, lacks = Obj(True), Obj(False)
> print("with ports: eager", fn(x, has)[0].item(), " compiled", opt_fn(x, has)[0].item())
> print("without ports: eager", fn(x, lacks)[0].item(), " compiled", opt_fn(x, lacks)[0].item())
> print("recompiled:", torch._dynamo.utils.counters["stats"]["unique_graphs"] > 1)
> ```
>
> Output:
>
> ```text
> with ports: eager 2.0 compiled 2.0
> without ports: eager 0.0 compiled 2.0
> recompiled: False
> ```
>
> The second call is wrong and there is no recompilation, no graph break and no
> warning. Swapping the call order gives the mirrored wrong answer, which confirms
> that whichever instance is seen first wins.
>
> ### Cause
>
> `BuiltinVariable.call_isinstance` in `torch/_dynamo/variables/builtin.py`
> evaluates the metaclass hook on the real object and builds a constant from the
> result:
>
> ```python
> if (
> isinstance(arg, variables.UserDefinedObjectVariable)
> and "__instancecheck__" in isinstance_type.__class__.__dict__
> ):
> return VariableTracker.build(
> tx,
> isinstance_type.__class__.__instancecheck__(isinstance_type, arg.value),
> )
> ```
>
> Running the hook is what makes Protocols work at all, so the call itself is not
> the problem. The problem is that the result is folded into the graph without
> recording what it depended on. For a data-member Protocol,
> `_ProtocolMeta.__instancecheck__` performs a `hasattr()` per member, and none of
> those lookups turn into guards.
>
> The same shape of hole applies to any metaclass `__instancecheck__` that reads
> instance state rather than the type, so a hypothetical fix should probably not
> be Protocol-specific.
>
> ### Possible directions
>
> 1. Install attribute-presence guards for the members a `runtime_checkable`
> Protocol inspects (`__protocol_attrs__`). Narrow and cheap, but only covers
> Protocols and reaches into `typing` internals.
> 2. Graph break instead of constant-folding when the classinfo has a hook that is
> not on a known-safe allowlist. Sound, but would regress code that relies on
> the current behavior, which is a significant amount given how long it has
> worked this way.
> 3. Keep folding, but treat the hook result as a value that needs its own guard
> mechanism.
>
> I do not have a strong opinion on which is right, hence this issue rather than a
> PR.
>
> ### Versions
>
> Reproduced on current main (2.15.0a0) and on the released 2.14.0, CPython 3.14,
> Windows. `backend="eager"` is used to show it is Dynamo-level; inductor behaves
> the same.
cc @ezyang @gchanan @kadeng @msaroufim @chauhang @penguinwu @voznesenskym @EikanWang @jgong5 @Guobing-Chen @XiaobingSuper @zhuhaozhe @blzheng @wenzhe-nrv @jiayisunx @amjames @jataylo @azahed98 @anijain2305 @williamwen42 @jansel
Contributor guide
Research direction
Start with the reproducer in the issue, then read BuiltinVariable.call_isinstance in torch/_dynamo/variables/builtin.py and trace how the Protocol __instancecheck__ reads instance attributes. Compare compiled and eager results for both call orders; done means differing instances produce correct results without silently sharing an unguarded constant-folded answer.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- python, pytorch
- Domain
- compilers, machine-learning
- Issue type
- Bug
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Active
- Clarity
- Mostly clear
- Newbie friendliness
- 38/100