[Flyout System] Prevent race condition crash when closing managed flyouts

packages/eui/changelogs/upcoming/9356.md

@@ -0,0 +1,3 @@
+**Bug fixes**
+
+- Fixed a potential crash in the flyout system: due to asynchronous state updates and React's batching behavior, it was possible to experience a crash when closing a managed flyout.

packages/eui/src/components/flyout/manager/flyout_managed.test.tsx

@@ -8,9 +8,19 @@
 
 /* eslint-disable @typescript-eslint/no-var-requires */
 
+jest.mock('react-dom', () => {
+  const actual = jest.requireActual('react-dom');
+  const mockFlushSync = jest.fn((callback: () => void) => callback());
+  return {
+    ...actual,
+    flushSync: mockFlushSync,
+  };
+});
+
 import React from 'react';
 import { act } from '@testing-library/react';
 import userEvent from '@testing-library/user-event';
+import * as ReactDOM from 'react-dom';
 
 import { render } from '../../../test/rtl';
 import { requiredProps } from '../../../test/required_props';
@@ -23,6 +33,8 @@ import {
   PROPERTY_LEVEL,
 } from './const';
 
+const mockFlushSync: jest.Mock = jest.mocked(ReactDOM.flushSync);
+
 // Mock base flyout to a simple div to avoid complex internals
 jest.mock('../flyout.component', () => {
   const React = require('react');
@@ -46,12 +58,15 @@ jest.mock('../flyout.component', () => {
 
 // Shared mock functions - must be defined in module scope for Jest
 const mockCloseFlyout = jest.fn();
-const createMockState = () => ({
+
+// Create mock state and functions once at module scope to avoid redundant object creation
+const mockState = {
   sessions: [],
   flyouts: [],
   layoutMode: 'side-by-side' as const,
-});
-const createMockFunctions = () => ({
+};
+
+const mockFunctions = {
   dispatch: jest.fn(),
   addFlyout: jest.fn(),
   closeFlyout: mockCloseFlyout,
@@ -60,14 +75,16 @@ const createMockFunctions = () => ({
   goBack: jest.fn(),
   goToFlyout: jest.fn(),
   historyItems: [],
-});
+};
+
+const mockFlyoutManager = {
+  state: mockState,
+  ...mockFunctions,
+};
 
 // Mock hooks that would otherwise depend on ResizeObserver or animation timing
 jest.mock('./hooks', () => ({
-  useFlyoutManager: () => ({
-    state: createMockState(),
-    ...createMockFunctions(),
-  }),
+  useFlyoutManager: () => mockFlyoutManager,
   useIsFlyoutActive: () => true,
   useHasChildFlyout: () => false,
   useParentFlyoutSize: () => 'm',
@@ -101,17 +118,17 @@ jest.mock('./validation', () => ({
 
 jest.mock('./provider', () => ({
   ...jest.requireActual('./provider'),
-  useFlyoutManager: () => ({
-    state: createMockState(),
-    ...createMockFunctions(),
-  }),
+  useFlyoutManager: () => mockFlyoutManager,
 }));
 
 // Mock resize observer hook to return a fixed width
 jest.mock('../../observer/resize_observer', () => ({
   useResizeObserver: () => ({ width: 480 }),
 }));
 
+// Cache the actual validation module for tests that need to temporarily restore it
+const actualValidation = jest.requireActual('./validation');
+
 describe('EuiManagedFlyout', () => {
   const renderInProvider = (ui: React.ReactElement) =>
     render(<EuiFlyoutManager>{ui}</EuiFlyoutManager>);
@@ -369,13 +386,10 @@ describe('EuiManagedFlyout', () => {
 
   describe('size handling', () => {
     it('defaults main flyout size to "m" when no size is provided', () => {
-      // Import the real validation function to test the actual behavior
-      const { validateManagedFlyoutSize } = jest.requireActual('./validation');
-
       // Temporarily restore the real validation function for this test
       const originalMock = require('./validation').validateManagedFlyoutSize;
       require('./validation').validateManagedFlyoutSize =
-        validateManagedFlyoutSize;
+        actualValidation.validateManagedFlyoutSize;
 
       const { getByTestSubject } = renderInProvider(
         <EuiManagedFlyout
@@ -395,13 +409,10 @@ describe('EuiManagedFlyout', () => {
     });
 
     it('defaults child flyout size to "s" when no size is provided', () => {
-      // Import the real validation function to test the actual behavior
-      const { validateManagedFlyoutSize } = jest.requireActual('./validation');
-
       // Temporarily restore the real validation function for this test
       const originalMock = require('./validation').validateManagedFlyoutSize;
       require('./validation').validateManagedFlyoutSize =
-        validateManagedFlyoutSize;
+        actualValidation.validateManagedFlyoutSize;
 
       const { getByTestSubject } = renderInProvider(
         <EuiManagedFlyout
@@ -595,5 +606,82 @@ describe('EuiManagedFlyout', () => {
       expect(mockCloseFlyout).toHaveBeenCalledWith('main-flyout');
       expect(onCloseMain).toHaveBeenCalled();
     });
+
+    it('uses flushSync to ensure synchronous state update before DOM cleanup', () => {
+      const onClose = jest.fn();
+
+      const { getByTestSubject } = renderInProvider(
+        <EuiManagedFlyout
+          id="flush-sync-test"
+          level={LEVEL_MAIN}
+          onClose={onClose}
+          flyoutMenuProps={{ title: 'Test Flyout' }}
+        />
+      );
+
+      // Clear any setup calls
+      mockFlushSync.mockClear();
+      mockCloseFlyout.mockClear();
+
+      // Trigger close via user interaction
+      act(() => {
+        userEvent.click(getByTestSubject('managed-flyout'));
+      });
+
+      // Verify flushSync was called
+      expect(mockFlushSync).toHaveBeenCalledTimes(1);
+      expect(mockFlushSync).toHaveBeenCalledWith(expect.any(Function));
+
+      // Verify closeFlyout was called (inside flushSync)
+      expect(mockCloseFlyout).toHaveBeenCalledWith('flush-sync-test');
+
+      // Verify onClose was called after the synchronous state update
+      expect(onClose).toHaveBeenCalled();
+    });
+
+    it('calls closeFlyout inside flushSync callback', () => {
+      const onClose = jest.fn();
+      const callOrder: string[] = [];
+
+      // Track execution order
+      mockFlushSync.mockImplementation((callback: () => void) => {
+        callOrder.push('flushSync-start');
+        callback();
+        callOrder.push('flushSync-end');
+      });
+
+      mockCloseFlyout.mockImplementation(() => {
+        callOrder.push('closeFlyout');
+      });
+
+      onClose.mockImplementation(() => {
+        callOrder.push('onClose');
+      });
+
+      const { getByTestSubject } = renderInProvider(
+        <EuiManagedFlyout
+          id="flush-sync-order-test"
+          level={LEVEL_MAIN}
+          onClose={onClose}
+          flyoutMenuProps={{ title: 'Test Flyout' }}
+        />
+      );
+
+      // Clear setup
+      callOrder.length = 0;
+
+      // Trigger close
+      act(() => {
+        userEvent.click(getByTestSubject('managed-flyout'));
+      });
+
+      // Verify closeFlyout is called INSIDE flushSync, and onClose is called AFTER
+      expect(callOrder).toEqual([
+        'flushSync-start',
+        'closeFlyout',
+        'flushSync-end',
+        'onClose',
+      ]);
+    });
   });
 });

packages/eui/src/components/flyout/manager/flyout_managed.tsx

@@ -6,7 +6,15 @@
  * Side Public License, v 1.
  */
 
-import React, { useEffect, useMemo, useRef, useState, forwardRef } from 'react';
+import React, {
+  useEffect,
+  useLayoutEffect,
+  useMemo,
+  useRef,
+  useState,
+  forwardRef,
+} from 'react';
+import { flushSync } from 'react-dom';
 import { useCombinedRefs, useEuiMemoizedStyles } from '../../../services';
 import { useEuiI18n } from '../../i18n';
 import { useResizeObserver } from '../../observer/resize_observer';
@@ -168,29 +176,31 @@ export const EuiManagedFlyout = forwardRef<HTMLElement, EuiManagedFlyoutProps>(
     // Track if flyout was ever registered to avoid false positives on initial mount
     const wasRegisteredRef = useRef(false);
 
-    // Register with flyout manager context when open, remove when closed
+    // Track flyoutExistsInManager in a ref to avoid dependency loop
+    // The cleanup function needs the current value but shouldn't cause re-runs
+    const flyoutExistsInManagerRef = useRef(flyoutExistsInManager);
+
+    // Update ref when flyoutExistsInManager changes
     useEffect(() => {
+      flyoutExistsInManagerRef.current = flyoutExistsInManager;
+    }, [flyoutExistsInManager]);
+
+    // Register with flyout manager context when open, remove when closed
+    // Using useLayoutEffect to run synchronously before DOM updates
+    useLayoutEffect(() => {
       addFlyout(flyoutId, title!, level, size as string);
 
       return () => {
         // Only call closeFlyout if it wasn't already called via onClose
         // This prevents duplicate removal when using Escape/X button
-        if (flyoutExistsInManager) {
+        if (flyoutExistsInManagerRef.current) {
           closeFlyout(flyoutId);
         }
 
         // Reset navigation tracking when explicitly closed via isOpen=false
         wasRegisteredRef.current = false;
       };
-    }, [
-      flyoutId,
-      title,
-      level,
-      size,
-      addFlyout,
-      closeFlyout,
-      flyoutExistsInManager,
-    ]);
+    }, [flyoutId, title, level, size, addFlyout, closeFlyout]);
 
     // Detect when flyout has been removed from manager state (e.g., via Back button)
     // and trigger onClose callback to notify the parent component
@@ -224,16 +234,6 @@ export const EuiManagedFlyout = forwardRef<HTMLElement, EuiManagedFlyoutProps>(
       }
     }, [currentSession, flyoutId, level]);
 
-    useEffect(() => {
-      return () => {
-        // Only remove from manager on component unmount if still registered
-        // This cleanup serves as a final safety net for edge cases
-        if (flyoutExistsInManager) {
-          closeFlyout(flyoutId);
-        }
-      };
-    }, [closeFlyout, flyoutId, flyoutExistsInManager]);
-
     // Track width changes for flyouts
     const { width } = useResizeObserver(isActive ? flyoutRef : null, 'width');
 
@@ -242,7 +242,10 @@ export const EuiManagedFlyout = forwardRef<HTMLElement, EuiManagedFlyoutProps>(
       // CRITICAL: Update manager state FIRST before allowing React to unmount
       // This prevents race conditions during portal → inline DOM transitions
       // and ensures cascade close logic runs before DOM cleanup begins
-      closeFlyout(flyoutId);
+      // Using flushSync to force synchronous state update completion
+      flushSync(() => {
+        closeFlyout(flyoutId);
+      });
 
       // trigger parent callback, unmounts the component
       if (onCloseCallbackRef.current) {