Skip to content

Fix race-condition during render#5187

Closed
piaskowyk wants to merge 5 commits intomainfrom
@piaskowyk/fix-race-condition-during-render
Closed

Fix race-condition during render#5187
piaskowyk wants to merge 5 commits intomainfrom
@piaskowyk/fix-race-condition-during-render

Conversation

@piaskowyk
Copy link
Member

@piaskowyk piaskowyk commented Oct 5, 2023

Summary

This PR fixes a problem with race conditions during updating parent view and mounting children. The problem occurs in some specific circumstances:

  • Parent has an animated style
  • Parent update style without animation, just assigns new value to shared value (single frame update, without animation)
  • Child doesn't have animated style
  • Child's styles depend on parent size
  • Child is mounting during update of parent style

In this situation (repro in attached example code) may happen race conditions. See at the graph:

image

Style updating by Reanimated

image

Mounting new view by React

image

So when we merge both graph in single one, It should behave in this way:

image

But because Reanimated is trying to perform update synchronously it looks like that:

image

In this specific situation, the React Shadow Tree and UI Native Tree are not synchronized. To resolve this issue, I disabled the synchronous style update through Reanimated until those trees can be synchronized once more.

before after
Screen.Recording.2023-10-05.at.14.43.58.mov
Screen.Recording.2023-10-05.at.14.40.24.mov

Fixes #4932

Test plan

Example code
import React, { useState } from 'react';
import Reanimated, {
  useAnimatedStyle,
  useSharedValue,
} from 'react-native-reanimated';
import {Pressable, View, StyleSheet, Text} from 'react-native';

function App() {
  const [toggle, setToggle] = useState(false);
  const height = useSharedValue(100);

  const animatedStyle = useAnimatedStyle(() => {
    return { height: height.value };
  });

  return (
    <View style={styles.container}>
      <Reanimated.View style={[styles.parent, animatedStyle]}>
      {toggle && <Reanimated.View style={[styles.child]} />}
      </Reanimated.View>

      <Pressable onPress={() => {
        setToggle(!toggle);
        height.value = height.value === 100 ? 200 : 100;
      }} style={styles.button}>
        <Text>Press Me</Text>
      </Pressable>

    </View>
  );
}

const styles = StyleSheet.create({
  container: {
    height: '100%',
  },
  parent: {
    backgroundColor: 'red',
    width: '100%',
  },
  child: {
    backgroundColor: 'green',
    height: '100%',
    width: '50%'
  },
  button: {
    position: 'absolute',
    bottom: 100,
  },
});

export default App;

#import <React/RCTUIManager.h>

@interface RCTUIManager (Reanimated)
@property (atomic) NSNumber *isViewTreesSynchronized;
Copy link
Member Author

Choose a reason for hiding this comment

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

It needs to be NSNumber, because I can't inject BOOL field with objc_setAssociatedObject method

@piaskowyk piaskowyk marked this pull request as ready for review October 5, 2023 12:32
@piaskowyk piaskowyk requested review from kmagiera and tomekzaw October 5, 2023 12:32
- (void)setIsViewTreesSynchronized:(NSNumber *)isViewTreesSynchronized
{
objc_setAssociatedObject(self, @selector(isViewTreesSynchronized), isViewTreesSynchronized, OBJC_ASSOCIATION_RETAIN);
}
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
}
}

{
objc_setAssociatedObject(self, @selector(isViewTreesSynchronized), isViewTreesSynchronized, OBJC_ASSOCIATION_RETAIN);
}
- (id)isViewTreesSynchronized
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
- (id)isViewTreesSynchronized
- (NSNumber *)isViewTreesSynchronized

}
BOOL canUpdateSynchronously = trySynchronously && ![strongSelf.uiManager hasEnqueuedUICommands];

auto isViewTreesSynchronized = [[strongSelf.uiManager isViewTreesSynchronized] intValue] == 1;
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
auto isViewTreesSynchronized = [[strongSelf.uiManager isViewTreesSynchronized] intValue] == 1;
auto isViewTreesSynchronized = [[strongSelf.uiManager isViewTreesSynchronized] boolValue];


- (void)setNeedsLayout;

- (id)isViewTreesSynchronized;
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
- (id)isViewTreesSynchronized;
- (NSNumber *)isViewTreesSynchronized;

@piaskowyk piaskowyk marked this pull request as draft October 12, 2023 09:00
@piaskowyk piaskowyk force-pushed the @piaskowyk/fix-race-condition-during-render branch from a7bfbb8 to f9d44b9 Compare October 12, 2023 09:03
@piaskowyk
Copy link
Member Author

Closed due to: #5224

@piaskowyk piaskowyk closed this Oct 12, 2023
@piaskowyk piaskowyk deleted the @piaskowyk/fix-race-condition-during-render branch November 15, 2023 15:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Child component does not resize properly if it remounts when parent changes dimensions

2 participants

Comments