Skip to content

Commit 6ce80c5

Browse files
phryneasj-piasecki
andauthored
fix a potential race on component unmount (#2262)
## Description This fixes a potential race condition where the component unmounted before the callback passed into `setImmediate` in `updateHandlers` could execute. That would lead to a "No handler for tag X" error, since the handler would not have been registered any more at that point in time. There might also be other race conditions because of that `setImmediate`, but this is the only one I could reproduce. ## Test plan I encountered this in our project code and debugged it with a [replay recording](https://www.replay.io/). I might be able to provide that replay if contacted on a private side channel for verification purposes, but I assume the problem & solution are probably already understandable just by the explanation above. Co-authored-by: Jakub Piasecki <jakubpiasecki67@gmail.com>
1 parent 97f6df9 commit 6ce80c5

File tree

1 file changed

+10
-3
lines changed

1 file changed

+10
-3
lines changed

src/handlers/gestures/GestureDetector.tsx

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import React, { useEffect, useRef } from 'react';
1+
import React, { useEffect, useRef, RefObject } from 'react';
22
import {
33
GestureType,
44
HandlerCallbacks,
@@ -226,7 +226,8 @@ function attachHandlers({
226226
function updateHandlers(
227227
preparedGesture: GestureConfigReference,
228228
gestureConfig: ComposedGesture | GestureType | undefined,
229-
gesture: GestureType[]
229+
gesture: GestureType[],
230+
mountedRef: RefObject<boolean>
230231
) {
231232
gestureConfig?.prepare();
232233

@@ -246,6 +247,9 @@ function updateHandlers(
246247
// and handlerTags in BaseGesture references should be updated in the loop above (we need to wait
247248
// in case of external relations)
248249
setImmediate(() => {
250+
if (!mountedRef.current) {
251+
return;
252+
}
249253
for (let i = 0; i < gesture.length; i++) {
250254
const handler = preparedGesture.config[i];
251255

@@ -575,6 +579,7 @@ export const GestureDetector = (props: GestureDetectorProps) => {
575579
const useReanimatedHook = gesture.some((g) => g.shouldUseReanimated);
576580
const viewRef = useRef(null);
577581
const firstRenderRef = useRef(true);
582+
const mountedRef = useRef(false);
578583
const webEventHandlersRef = useRef<WebEventHandler>({
579584
onGestureHandlerEvent: (e: HandlerStateChangeEvent<unknown>) => {
580585
onGestureHandlerEvent(e.nativeEvent);
@@ -619,6 +624,7 @@ export const GestureDetector = (props: GestureDetectorProps) => {
619624

620625
useEffect(() => {
621626
firstRenderRef.current = true;
627+
mountedRef.current = true;
622628
const viewTag = findNodeHandle(viewRef.current) as number;
623629

624630
validateDetectorChildren(viewRef.current);
@@ -631,6 +637,7 @@ export const GestureDetector = (props: GestureDetectorProps) => {
631637
});
632638

633639
return () => {
640+
mountedRef.current = false;
634641
dropHandlers(preparedGesture);
635642
};
636643
}, []);
@@ -650,7 +657,7 @@ export const GestureDetector = (props: GestureDetectorProps) => {
650657
webEventHandlersRef,
651658
});
652659
} else {
653-
updateHandlers(preparedGesture, gestureConfig, gesture);
660+
updateHandlers(preparedGesture, gestureConfig, gesture, mountedRef);
654661
}
655662
} else {
656663
firstRenderRef.current = false;

0 commit comments

Comments
 (0)