dequelabs / dequelabs/cauldron

ActionMenu/Listbox can enter an unbounded render loop under CPU load when `focusStrategy="first"`

Closed Beginner friendly
#2,512 0 comments 0 reactions 0 assignees View on GitHub
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

Open the contributing 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.