Skip to content
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.

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

Fix race condition in ResponderTouchHistoryStore #6278

Closed
wants to merge 1 commit into from
Closed

Fix race condition in ResponderTouchHistoryStore #6278

wants to merge 1 commit into from

Conversation

marcshilling
Copy link

On a cold boot of my React Native app, if the first touch on the screen is a scroll on a ListView/ScrollView, there appears to be a race condition and recordMoveTouchData is called before recordStartTouchData, therefore touchHistory.touchBank is not properly initialized.

Upon the first call,
var touchTrack = touchBank[touch.identifier];
touchTrack is undefined here, causing the crash. This code change resolves the problem by properly initializing the touchBank. Not sure if this is the correct approach, because it's probably an indication of a deeper problem, but wanted to start a conversation about it.

@jimfb
Copy link
Contributor

jimfb commented Mar 16, 2016

It seems to me that this is not the correct fix. React-native shouldn't be sending a move event before a start event, right?

@marcshilling
Copy link
Author

That's the exactly what I was thinking, but...I don't know.

@jimfb
Copy link
Contributor

jimfb commented Mar 16, 2016

I'm going to close this out, because I don't think this is the right fix. It would just hide the problem, making it more difficult to debug/reproduce/witness. Also, with this fix, you would still be loosing events, which is a bad user experience. I'm going to close this out in favor of tracking facebook/react-native#5246. If the RN team bounces it back here, we can re-investigate.

@jimfb jimfb closed this Mar 16, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants