diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index fdbfcec..18193a8 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -776,24 +776,22 @@ this repo. ### Open -#### `LayoutFurniImageView` / `LayoutAvatarImageView` — async fetch race - -In both files an effect kicks off an async `processAsImageUrl` / -`generateImage` and writes the result via `setImageElement`. If props -change twice in quick succession, the first fetch can resolve **after** -the second one and overwrite the newer image with the older one. - -**Fix shape**: capture a request-id ref at the start of the effect, only -write the result if the ref hasn't been bumped meanwhile. Or — better — -once React Query (#2) is enabled, model the image fetch as a query keyed -on the props tuple; React Query handles cancellation and ordering for -free. - -**Severity**: visible only on slow connections / rapid prop changes. Not -data-corrupting. +_(none — both previously-listed bugs have landed; see "Recently fixed" +below.)_ ### Recently fixed (in this branch) +- **`LayoutFurniImageView` / `LayoutAvatarImageView` async fetch race + fixed.** Both effects kicked off async image work + (`TextureUtils.generateImage` / SDK `resetFigure` callback) and wrote + the result via `setImageElement` / `setAvatarUrl` guarded only by an + `isMounted` / `isDisposed` ref. If props changed twice in quick + succession the older fetch could resolve last and overwrite the + newer image. Both now capture a `requestIdRef` bumped at the start + of the effect; the async callback bails when its captured id no + longer matches the latest one. (React Query keyed on the props + tuple would also work, but neither call goes through a composer / + parser pair so the request-id ref is the lighter fix.) - **`MainView` CREATED/ENDED race fixed.** Two independent `useNitroEvent` listeners on `RoomSessionEvent.CREATED` / `RoomSessionEvent.ENDED` could land out of order under flaky diff --git a/src/common/layout/LayoutAvatarImageView.tsx b/src/common/layout/LayoutAvatarImageView.tsx index 853d361..75233a8 100644 --- a/src/common/layout/LayoutAvatarImageView.tsx +++ b/src/common/layout/LayoutAvatarImageView.tsx @@ -20,6 +20,12 @@ export const LayoutAvatarImageView: FC = props => const [ avatarUrl, setAvatarUrl ] = useState(null); const [ isReady, setIsReady ] = useState(false); const isDisposed = useRef(false); + // Request id bumped on every prop change. The SDK can call + // resetFigure asynchronously when server-side figure data lands; + // if props change in quick succession the older callback could + // otherwise overwrite the newer image. The closure captures the + // id and bails when stale. + const requestIdRef = useRef(0); const getClassNames = useMemo(() => { @@ -52,6 +58,7 @@ export const LayoutAvatarImageView: FC = props => { if(!isReady) return; + const requestId = ++requestIdRef.current; const figureKey = [ figure, gender, direction, headOnly ].join('-'); if(AVATAR_IMAGE_CACHE.has(figureKey)) @@ -62,7 +69,7 @@ export const LayoutAvatarImageView: FC = props => { const resetFigure = (_figure: string) => { - if(isDisposed.current) return; + if(isDisposed.current || (requestIdRef.current !== requestId)) return; const avatarImage = GetAvatarRenderManager().createAvatarImage(_figure, AvatarScaleType.LARGE, gender, { resetFigure: (figure: string) => resetFigure(figure), dispose: null, disposed: false }); @@ -74,7 +81,7 @@ export const LayoutAvatarImageView: FC = props => const imageUrl = avatarImage.processAsImageUrl(setType); - if(imageUrl && !isDisposed.current) + if(imageUrl && !isDisposed.current && (requestIdRef.current === requestId)) { if(!avatarImage.isPlaceholder()) { diff --git a/src/common/layout/LayoutFurniImageView.tsx b/src/common/layout/LayoutFurniImageView.tsx index 8e456b9..c2ad62b 100644 --- a/src/common/layout/LayoutFurniImageView.tsx +++ b/src/common/layout/LayoutFurniImageView.tsx @@ -17,6 +17,11 @@ export const LayoutFurniImageView: FC = props => const { productType = 's', productClassId = -1, direction = 2, extraData = '', scale = 1, style = {}, ...rest } = props; const [ imageElement, setImageElement ] = useState(null); const isMounted = useRef(true); + // Request id bumped by the effect on every prop change. The async + // generateImage / imageReady callbacks capture it and only write + // back if it still matches — prevents an older, slower fetch from + // overwriting a newer one when props change in quick succession. + const requestIdRef = useRef(0); useEffect(() => { @@ -28,13 +33,13 @@ export const LayoutFurniImageView: FC = props => }; }, []); - const updateImage = useCallback(async (texture: any) => + const updateImage = useCallback(async (texture: any, requestId: number) => { if(!texture) return; const image = await TextureUtils.generateImage(texture); - if(image && isMounted.current) setImageElement(image); + if(image && isMounted.current && (requestIdRef.current === requestId)) setImageElement(image); }, []); const getStyle = useMemo(() => @@ -62,12 +67,14 @@ export const LayoutFurniImageView: FC = props => useEffect(() => { + const requestId = ++requestIdRef.current; + setImageElement(null); let imageResult: ImageResult = null; const listener: IGetImageListener = { - imageReady: (result) => updateImage(result?.data), + imageReady: (result) => updateImage(result?.data, requestId), imageFailed: null }; @@ -81,7 +88,7 @@ export const LayoutFurniImageView: FC = props => break; } - if(imageResult?.data) updateImage(imageResult.data); + if(imageResult?.data) updateImage(imageResult.data, requestId); }, [ productType, productClassId, direction, extraData, updateImage ]); return ;