react-native: [Android Crash] Touch data should have been recorded on start

Great data! I’ve chatted with @tadeuzagallo and we figured out there is indeed a race condition in how touches are processed.

For (perf) reasons, topTouchStart is immediately dispatched on js thread, while topTouchMove waits for js to collect it in at the beggining of a frame. This can lead into a situation in which they arrive to js in a wrong order, something like this:

I’ll fix it. Thanks for digging into this, @skevy & @marcshilling!

This also makes me think this github issue actually contains two different bugs that surface with the same redbox.

@majak Marc and I did a bit more research on this:

2016-03-17 12:22:28.850 DiscoveryVR - Dev[44643:4871099] OUTSIDE BLOCK: Module RCTEventEmitter, Method receiveTouches, Args (
    topTouchStart,
        (
                {
            identifier = 1;
            locationX = 226;
            locationY = "36.5";
            pageX = 242;
            pageY = "594.5";
            target = 189;
            timestamp = "65236062.87420401";
        }
    ),
        (
        0
    )
)
2016-03-17 12:22:28.873 DiscoveryVR - Dev[44643:4871447] OUTSIDE BLOCK: Module RCTEventEmitter, Method receiveTouches, Args (
    topTouchMove,
        (
                {
            identifier = 1;
            locationX = 226;
            locationY = "34.5";
            pageX = 242;
            pageY = "592.5";
            target = 189;
            timestamp = "65236079.616078";
        }
    ),
        (
        0
    )
)
2016-03-17 12:22:28.873 DiscoveryVR - Dev[44643:4871447] INSIDE BLOCK: Module RCTEventEmitter, Method receiveTouches, Args (
    topTouchMove,
        (
                {
            identifier = 1;
            locationX = 226;
            locationY = "34.5";
            pageX = 242;
            pageY = "592.5";
            target = 189;
            timestamp = "65236079.616078";
        }
    ),
        (
        0
    )
)
2016-03-17 12:22:28.874 [error][tid:com.facebook.React.JavaScript] Touch data should have been recorded on start
2016-03-17 12:22:28.879 [fatal][tid:com.facebook.React.RCTExceptionsManagerQueue] Unhandled JS Exception: Touch data should have been recorded on start
2016-03-17 12:22:28.879 DiscoveryVR - Dev[44643:4871447] INSIDE BLOCK: Module RCTEventEmitter, Method receiveTouches, Args (
    topTouchStart,
        (
                {
            identifier = 1;
            locationX = 226;
            locationY = "36.5";
            pageX = 242;
            pageY = "594.5";
            target = 189;
            timestamp = "65236062.87420401";
        }
    ),
        (
        0
    )
)

Here, we’re logging calls to the bridge with the method “receiveTouches”, referring to the method that receives touches in ReactNativeEventEmitter. In the log above, you notice we’re logging “OUTSIDE BLOCK” and “INSIDE BLOCK” – these logs are referring to:

OUTSIDE BLOCK: https://github.com/facebook/react-native/blob/master/React/Base/RCTBatchedBridge.m#L696 INSIDE BLOCK: https://github.com/facebook/react-native/blob/master/React/Base/RCTBatchedBridge.m#L705

As you can see in the log, the “OUTSIDE_BLOCK” stuff is executing in the proper order – a topTouchStart followed by a topTouchMove.

INSIDE_BLOCK, however, is executing in the wrong order. You can see the topTouchStart event doesn’t ACTUALLY get executed on the bridge until after the the topTouchMove.

This seems weird, but I think I may have a fundamental misunderstanding of how the JSC executor works. I was under the impression that while timing of a given JS call couldn’t be guaranteed (due to the async nature of the bridge), the order could be, because as the method name implies, updates are “enqueued”.

Obviously this could be fixed in JS by queuing up move calls until a touchStart is received, and we can enforce the order there. Perhaps the right fix, and presumably that would then be fixed on both platforms.

I’m curious though if anyone can explain the executor here though, and why order can’t be guaranteed.

CCing @javache here as well.

To be clear, we believe the issue on iOS has been fixed. Those commits don’t affect Android.

Changes that should fix this issue were just pushed on master. Commits from 8f07b01ac81ca5962a48ed996d616a065622fb24 to 31bb85a210e8a6edba4f59df6eb36ccc5cc58d9e. The important one with deeper explanation is 1d3db4c5dc8763d16f2d051fdf04a2976c0fb154. Since I didn’t repro this issue locally I cannot guarantee it will fix it for real. Therefore it would be great if you could patch it locally and see whether it really helps.

But also turn off chrome debugging please. Looks like this stack breaks it 😅. I’m looking into that too.