Skip to content

Fix: handle Firefox event loop bug causing 'Should not already be working' error #703

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
everettbu wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Fix: handle Firefox event loop bug causing 'Should not already be working' error #703

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 12 additions & 2 deletions packages/react-reconciler/src/ReactFiberWorkLoop.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1120,7 +1120,12 @@
forceSync: boolean,
): void {
if ((executionContext & (RenderContext | CommitContext)) !== NoContext) {
throw new Error('Should not already be working.');
// Fix for Firefox event loop bug where alert/debugger don't block MessageChannel events
// Schedule the work to run on the next event loop iteration instead of throwing
Scheduler_scheduleCallback(NormalSchedulerPriority, () => {
performWorkOnRoot(root, lanes, forceSync);
});
Comment on lines +1125 to +1127
Copy link
Copy Markdown

@greptile-apps greptile-apps bot Mar 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Original work priority is silently downgraded to NormalSchedulerPriority

performWorkOnRoot is called with whatever lane priorities the scheduler originally assigned (potentially SyncLane, InputContinuousLane, etc.), but this rescheduled retry always uses NormalSchedulerPriority. If the work was synchronous (e.g., forceSync === true or includes a blocking lane), running it at normal priority after the current work finishes could violate React's priority ordering guarantees — for example, a user-initiated synchronous update could be deferred behind lower-priority idle work that is already queued.

The same concern applies to the completeRoot reschedule at line 3522.

Prompt To Fix With AI 
This is a comment left during a code review.
Path: packages/react-reconciler/src/ReactFiberWorkLoop.js
Line: 1125-1127

Comment:
**Original work priority is silently downgraded to `NormalSchedulerPriority`**

`performWorkOnRoot` is called with whatever lane priorities the scheduler originally assigned (potentially `SyncLane`, `InputContinuousLane`, etc.), but this rescheduled retry always uses `NormalSchedulerPriority`. If the work was synchronous (e.g., `forceSync === true` or includes a blocking lane), running it at normal priority after the current work finishes could violate React's priority ordering guarantees — for example, a user-initiated synchronous update could be deferred behind lower-priority idle work that is already queued.

The same concern applies to the `completeRoot` reschedule at line 3522.

How can I resolve this? If you propose a fix, please make it concise.

Fix in Claude Code Fix in Codex

return;
}
Comment on lines 1122 to 1129
Copy link
Copy Markdown

@greptile-apps greptile-apps bot Mar 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Potential infinite re-scheduling loop

If the executionContext is stuck with RenderContext | CommitContext set (e.g., due to a genuine bug rather than the Firefox MessageChannel race), this callback will reschedule itself on every tick indefinitely. The original throw was a hard stop that surfaced this invariant violation immediately; the new approach silently swallows it and spins forever.

Consider adding a retry counter or a flag to bail out after a reasonable number of retries:

let reentryRetries = 0;
const MAX_REENTRY_RETRIES = 3;

if ((executionContext & (RenderContext | CommitContext)) !== NoContext) {
  if (reentryRetries++ < MAX_REENTRY_RETRIES) {
    Scheduler_scheduleCallback(NormalSchedulerPriority, () => {
      reentryRetries = 0;
      performWorkOnRoot(root, lanes, forceSync);
    });
  } else {
    reentryRetries = 0;
    throw new Error('Should not already be working.');
  }
  return;
}

This preserves the Firefox workaround for genuine scheduler race conditions while still failing loudly if the re-entry is a real bug. The same concern applies to the corresponding change in completeRoot (line 3519).

Prompt To Fix With AI 
This is a comment left during a code review.
Path: packages/react-reconciler/src/ReactFiberWorkLoop.js
Line: 1122-1129

Comment:
**Potential infinite re-scheduling loop**

If the `executionContext` is stuck with `RenderContext | CommitContext` set (e.g., due to a genuine bug rather than the Firefox MessageChannel race), this callback will reschedule itself on every tick indefinitely. The original `throw` was a hard stop that surfaced this invariant violation immediately; the new approach silently swallows it and spins forever.

Consider adding a retry counter or a flag to bail out after a reasonable number of retries:

```
let reentryRetries = 0;
const MAX_REENTRY_RETRIES = 3;

if ((executionContext & (RenderContext | CommitContext)) !== NoContext) {
  if (reentryRetries++ < MAX_REENTRY_RETRIES) {
    Scheduler_scheduleCallback(NormalSchedulerPriority, () => {
      reentryRetries = 0;
      performWorkOnRoot(root, lanes, forceSync);
    });
  } else {
    reentryRetries = 0;
    throw new Error('Should not already be working.');
  }
  return;
}
```

This preserves the Firefox workaround for genuine scheduler race conditions while still failing loudly if the re-entry is a real bug. The same concern applies to the corresponding change in `completeRoot` (line 3519).

How can I resolve this? If you propose a fix, please make it concise.

Fix in Claude Code Fix in Codex


if (enableProfilerTimer && enableComponentPerformanceTrack) {
Expand Down Expand Up @@ -3512,7 +3517,12 @@
flushRenderPhaseStrictModeWarningsInDEV();

if ((executionContext & (RenderContext | CommitContext)) !== NoContext) {
throw new Error('Should not already be working.');
// Fix for Firefox event loop bug where alert/debugger don't block MessageChannel events
// Schedule the work to run on the next event loop iteration instead of throwing
Scheduler_scheduleCallback(NormalSchedulerPriority, () => {
performWorkOnRoot(root, lanes, forceSync);

Check failure on line 3523 in packages/react-reconciler/src/ReactFiberWorkLoop.js

View workflow job for this annotation

GitHub Actions / Run eslint 

'forceSync' is not defined
});
Comment on lines +3522 to +3524
Copy link
Copy Markdown

@greptile-apps greptile-apps bot Mar 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

forceSync is not in scope — will throw ReferenceError at runtime

forceSync is a parameter of performWorkOnRoot, not of completeRoot. Referencing it inside this closure in completeRoot will cause a ReferenceError: forceSync is not defined whenever this code path is triggered. This appears to be a direct copy-paste from the performWorkOnRoot change without adapting the argument list.

completeRoot does not take a forceSync parameter, so there is no obvious single value to pass here. The caller of completeRoot is what knows whether the work is forced-sync. One option is to thread forceSync down as an additional parameter to completeRoot, or alternatively to always pass false (concurrent) and accept a possible downgrade in urgency. Either way, the current code cannot execute correctly.

Suggested change
Scheduler_scheduleCallback(NormalSchedulerPriority, () => {
performWorkOnRoot(root, lanes, forceSync);
});
Scheduler_scheduleCallback(NormalSchedulerPriority, () => {
performWorkOnRoot(root, lanes, false);
});

(Note: passing false is a conservative placeholder — the correct value of forceSync needs to be determined from the call-site context.)

Prompt To Fix With AI 
This is a comment left during a code review.
Path: packages/react-reconciler/src/ReactFiberWorkLoop.js
Line: 3522-3524

Comment:
**`forceSync` is not in scope — will throw `ReferenceError` at runtime**

`forceSync` is a parameter of `performWorkOnRoot`, not of `completeRoot`. Referencing it inside this closure in `completeRoot` will cause a `ReferenceError: forceSync is not defined` whenever this code path is triggered. This appears to be a direct copy-paste from the `performWorkOnRoot` change without adapting the argument list.

`completeRoot` does not take a `forceSync` parameter, so there is no obvious single value to pass here. The caller of `completeRoot` is what knows whether the work is forced-sync. One option is to thread `forceSync` down as an additional parameter to `completeRoot`, or alternatively to always pass `false` (concurrent) and accept a possible downgrade in urgency. Either way, the current code cannot execute correctly.

```suggestion
    Scheduler_scheduleCallback(NormalSchedulerPriority, () => {
      performWorkOnRoot(root, lanes, false);
    });
```
_(Note: passing `false` is a conservative placeholder — the correct value of `forceSync` needs to be determined from the call-site context.)_

How can I resolve this? If you propose a fix, please make it concise.

Fix in Claude Code Fix in Codex

return;
}

if (enableProfilerTimer && enableComponentPerformanceTrack) {
Expand Down
Loading