dequelabs / dequelabs/cauldron
ActionMenu/Listbox can enter an unbounded render loop under CPU load when `focusStrategy="first"`
- Dominant language
- TypeScript
- Stars
- 127
- Forks
- 31
- Avg merge
- 2d 12h
- Merged PRs (30d)
- 8
Description
## Summary
`Listbox`, `ActionList`, and `ActionMenu` keep the active option in sync through a two-way mirror. Under normal timing this mirror settles after one pass. Under heavy CPU load it can fail to settle and instead loop without end. React then prints thousands of `Maximum update depth exceeded` warnings and the process can run out of memory.
This affects `@deque/cauldron-react`. We saw it in 7.2.0. It is still present in the current release, 7.4.0.
## Where the bug is
The active option is stored in two places at once and each copy updates the other.
- `ActionList` holds its own `activeOption` state and passes it to `Listbox` as a controlled prop. It updates that state from `Listbox` through the `onActiveChange` callback. See `packages/react/src/components/ActionList/ActionList.tsx:32`, `:81`, and `:83`.
- `Listbox` copies the controlled prop into its own internal `activeOption` state. It has two effects that keep the two copies in sync. One effect reports its own active option to the parent through `onActiveChange`. The other effect copies the parent's prop into its own state when the two differ. See `packages/react/src/components/Listbox/Listbox.tsx:155` and `:161`.
- `Listbox`'s focus handler for `focusStrategy === 'first'` calls `setActiveOption(firstOption)` on every focus event. It does not first check whether `firstOption` is already the active option. See `packages/react/src/components/Listbox/Listbox.tsx:300`.
- `ActionMenu` sets `focusStrategy` to `'first'` by default and passes it down on every open. See `packages/react/src/components/ActionMenu/ActionMenu.tsx:84` and `:203`.
## Root cause
The focus handler has no "already active" early return. Each focus event writes the active option again. That write makes `Listbox` report the option to the parent. The parent re-renders and passes the option back down as a prop. The sync effect then copies that prop into `Listbox`'s state again.
When timing is normal, the sync effect sees the same element and stops, so the loop settles after one pass. When the CPU is saturated, the two effects interleave in an order where the two copies never reach a matching state, so the updates repeat without end. React's update-depth guard fires and the process eventually runs out of memory.
We reproduced this three times, each time independently, and captured about 14,600 warning stacks. Every stack ran through the library's own components, not through any of our code. All of the stacks followed the same call path, which points to a single cause.
Limiting the number of test processes that run at once stops the loop. We verified this. It confirms the loop is driven by concurrent load.
## How it was found
We hit this in a test suite in another repo. One test file that mounts `ActionMenu` failed at random. Only that one file failed. The rest of the suite was unaffected. That file passed every time when we ran it alone. It only failed when enough test processes ran at once to saturate the CPU. On a busy machine the failing process printed more than 13,000 `Maximum update depth exceeded` warnings over about three minutes and then died with a V8 heap out-of-memory error.
The failure is a scheduler-timing race, so machines with more cores hit it more often.
## Reproduction
The loop is a timing race, so it does not reproduce in a small or isolated test. It needs many test processes competing for the CPU at once.
- Run a large test suite that mounts `ActionMenu` on a machine with many cores.
- Or run `node --test` with `--test-concurrency=20` over a large `*.test.{ts,tsx}` glob. This reproduces the loop reliably, usually in under two minutes, with the failure at about 135 seconds. `node:test` only spawns as many worker processes as files given, so a small subset will not reproduce it.
## Ruled out
We eliminated these as causes:
- Prop object identity. Memoizing the props passed to `ActionMenu` does not help.
- Trigger ref forwarding.
- The number of test files in the suite. The crash reproduced with a smaller set of files too. The count of files only changes how often the race fires, not whether the bug exists.
There is no consumer-side workaround. The props needed to break the loop from outside are not exposed by `ActionMenu`.
## Suggested fix
Add an "already active" check in `Listbox`'s focus handler so it does not call `setActiveOption` when the target option is already the active one. That removes the repeated write that starts the loop. Apply this check in both the `focusStrategy === 'first'` branch and the `focusStrategy === 'last'` branch at `packages/react/src/components/Listbox/Listbox.tsx:300`. Both branches write on every focus event without checking, so both need the fix. `ActionMenu` triggers the `'first'` branch, but the `'last'` branch has the same bug.
Contributor guide
Research direction
Start in packages/react/src/components/Listbox/Listbox.tsx at the focusStrategy branches around line 300, then read the active-option synchronization effects around lines 155 and 161. Compare how ActionList and ActionMenu pass and update the value in packages/react/src/components/ActionList/ActionList.tsx and ActionMenu/ActionMenu.tsx. Done means repeated focus events no longer trigger redundant active-option updates in either branch; exercise the large node:test run with --test-concurrency=20 to check that the render loop and warning storm do not recur.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- react, typescript
- Domain
- frontend
- Issue type
- Bug
- Difficulty
- 2/5
- Estimated time
- 1-3 hours
- Activity status
- Active
- Clarity
- Clearly specified
- Newbie friendliness
- 78/100