Skip to content

Comments

Fix race condition in View#render by removing shared mutable state#1499

Open
botandrose wants to merge 1 commit intohotwired:mainfrom
botandrose:fix-renderer-race-condition
Open

Fix race condition in View#render by removing shared mutable state#1499
botandrose wants to merge 1 commit intohotwired:mainfrom
botandrose:fix-renderer-race-condition

Conversation

@botandrose
Copy link
Contributor

@botandrose botandrose commented Feb 5, 2026

Hi folks! I'm back with a small concurrency fix.

Problem

The render method in View stores this.renderer so delegates can mutate it, then accesses this.renderer.renderMethod after async operations. A concurrent render can delete this.renderer, raising: TypeError: can't access property "renderMethod", this.renderer is undefined, resulting in the the first render failing entirely.

Root Cause

  1. Render A starts, sets this.renderer = rendererA
  2. Render A awaits renderSnapshot()
  3. While A is paused, Render B starts and overwrites this.renderer = rendererB
  4. Render B completes and its finally block runs delete this.renderer
  5. Render A resumes and tries to access this.renderer.renderMethod, and boom, undefined

Prognosis

This isn't a critical bug, because it just means a different render wins the race condition than would otherwise. Perhaps its even weirdly desirable as an accidental "optimisation" due to less work being done. But it adds noise to my error reporting with semi-frequent exceptions (a few a day). Mostly, I think shared mutable state in parallel operations is better avoided, if it can be... it's hard to reason about, and could result in a more serious bug in the future that's difficult to track down.

Solution

Remove the this.renderer shared mutable state entirely. Instead of having delegates mutate shared state, have allowsImmediateRender return the (possibly modified) render function. Any changes to the other properties of this.renderer were not used, so just returning the render function is sufficient to retain the existing behavior.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants