fix: avoid false infinite loop detection when cached signal self-updates#24412
fix: avoid false infinite loop detection when cached signal self-updates#24412Artur- wants to merge 7 commits into
Conversation
When an effect reads from a cached signal that detects its value is stale, the cached signal recomputes and submits an update. This previously triggered infinite loop detection because the notification happened while the effect was still active. However, this is lazy evaluation (read-triggered), not an actual write loop. Add a ThreadLocal flag to track when we're in a "read-triggered update" context and skip the infinite loop check in that case. Fixes #24409 https://claude.ai/code/session_0193Z8ydgjUCCpbgCZwHhGGM
|
|
|
|
Legioth
left a comment
Legioth
left a comment
There was a problem hiding this comment.
I'm far from sure, but it feels like this flag is too eager since it also applies to any actual loop that is triggered as a consequence of the cache read. It's only interactions at the boundary between the reader and the inside of the read that should be ignored but now it also ignores anything between effects inside the read.
|
One alternative could be to stash away the contents of |
Address review feedback: instead of using a global flag that skips loop detection for ALL effects during the cached read, stash and restore activeEffects. This ensures: - The reading effect won't see itself as active during the cache update - Effects that run inside the cached read still have proper loop detection https://claude.ai/code/session_0193Z8ydgjUCCpbgCZwHhGGM
Add a unit test for `Effect.runWithStashedActiveEffects` confirming that an effect created and self-looping inside the stash is still caught by infinite-loop detection. This protects against regressing back to a coarse "skip-all" approach (e.g. a global flag) that would mask real loops happening inside the cached signal's update cascade.
|
Legioth
dismissed
their stale review
Approach seems sensible. I'm removing myself as a reviewer since I don't expect to prioritize having a closer look during the upcoming week.
3 participants
Summary
ThreadLocalflag inEffectto track when we're in a "read-triggered update" contextsubmit()call in this contextBackground
When an effect reads from a cached signal that detects its value is stale (via
hasChanges()), the cached signal recomputes its value and submits an update. This update triggers a change notification back to the effect. Previously, this was incorrectly detected as an infinite loop because the effect was still active when the notification arrived.However, this is not a true infinite loop - it's lazy evaluation happening during a read operation. The effect is reading, not writing, so there's no actual loop.
Test plan
infiniteLoopDetection_cachedSignalSelfUpdate_notDetectedAsLoopto verify an effect reading a cached signal that self-updates does not throw an infinite loop exceptionFixes #24409
https://claude.ai/code/session_0193Z8ydgjUCCpbgCZwHhGGM
Generated by Claude Code