wix-incubator / DetoxSync

Synchronization framework for Detox and other testing frameworks
MIT License
34 stars 16 forks source link

Fix possible segfault with Reanimated #71

Closed tyronet-sportsbet closed 6 months ago

tyronet-sportsbet commented 7 months ago

This is my attempt to fix a segfault we were seeing in Detox. For reference, the segfault stack trace looks like this:

Stack trace The app has crashed, see the details below: Signal 11 was raised ( 0 Detox 0x0000000101a74bb8 +[NSThread(DetoxUtils) dtx_demangledCallStackSymbols] + 36 1 Detox 0x0000000101a77758 __DTXHandleCrash + 440 2 Detox 0x0000000101a77d88 __DTXHandleSignal + 72 3 libsystem_platform.dylib 0x00000001cc0a7c80 _sigtramp + 52 4 DetoxSync 0x00000001028c7ce0 -[_DTXTimerTrampoline setDisplayLink:] + 96 5 DetoxSync 0x00000001028c7ce0 -[_DTXTimerTrampoline setDisplayLink:] + 96 6 DetoxSync 0x00000001028c54b8 +[DTXTimerSyncResource _timerProxyWithDisplayLink:] + 92 7 DetoxSync 0x00000001028c5568 +[DTXTimerSyncResource existingTimerProxyWithDisplayLink:create:] + 152 8 DetoxSync 0x00000001028bb29c +[DTXSyncManager trackDisplayLink:name:] + 84 9 DetoxSync 0x00000001028ae850 -[NSObject(REANodesManagerDTXSpy) __detox_sync_startUpdatingOnAnimationFrame] + 104 10 Sportsbet Beta 0x0000000100fc5728 RNGHHitSlopInsetRect + 71804 11 Sportsbet Beta 0x0000000100fc5698 RNGHHitSlopInsetRect + 71660 12 Sportsbet Beta 0x0000000100fc692c RNGHHitSlopInsetRect + 76416 13 Sportsbet Beta 0x0000000100fc4fec RNGHHitSlopInsetRect + 69952 14 Sportsbet Beta 0x00000001010456ac RCTSurfaceStageIsPreparing + 32436 15 Sportsbet Beta 0x000000010104579c RCTSurfaceStageIsPreparing + 32676 16 DetoxSync 0x00000001028afc08 ____detox_sync_dispatch_wrapper_block_invoke + 44 17 libdispatch.dylib 0x000000018010d244 _dispatch_call_block_and_release + 24 18 libdispatch.dylib 0x000000018010ea98 _dispatch_client_callout + 16 19 libdispatch.dylib 0x000000018011c41c _dispatch_main_queue_drain + 976 20 libdispatch.dylib 0x000000018011c03c _dispatch_main_queue_callback_4CF + 40 21 CoreFoundation 0x0000000180361c2c __CFRUNLOOP_IS_SERVICING_THE_MAIN_DISPATCH_QUEUE__ + 12 22 CoreFoundation 0x000000018035c0b0 __CFRunLoopRun + 2432 23 CoreFoundation 0x000000018035b218 CFRunLoopRunSpecific + 572 24 GraphicsServices 0x000000018c25f60c GSEventRunModal + 160 25 UIKitCore 0x0000000184d88a98 -[UIApplication _run] + 992 26 DetoxSync 0x00000001028b6084 __detox_sync_UIApplication_run + 376 27 UIKitCore 0x0000000184d8d634 UIApplicationMain + 112 28 Sportsbet Beta 0x0000000100e6d054 main + 124 29 dyld 0x00000001019e1cd8 start_sim + 20 30 ??? 0x0000000101c61f28 0x0 + 4324728616 31 ??? 0x8e27800000000000 0x0 + -8203447458743713792

This was using Detox 20.17.0, Xcode 14.1, React Native 0.68.1, react-native-reanimated 1.13.4.

Anyway to explain the change, if you look in react-native-reanimated master branch or 1.13.4 in either case this method that DetoxSync is hooking startUpdatingOnAnimationFrame will lazily create a CADisplayLink instance if one does not exist. But in DetoxSync as it stands today, it calls [self valueForKey:@"displayLink"] before it calls the upstream -startUpdatingOnAnimationFrame: from react-native-reanimated. This means it's possible the CADisplayLink instance will not have been created yet, so the value returned is just some dangling pointer!

This tiny PR just changes it so upstream -startUpdatingOnAnimationFrame: from react-native-reanimated is called first, before DetoxSync attempts to call [self valueForKey:@"displayLink"], ensuring that it's always lazily instantiated. I've tested it against our app and it fixes the segfault.

asafkorem commented 6 months ago

Hi @tyronet-sportsbet, I'll review this later today.

asafkorem commented 6 months ago

You're right @tyronet-sportsbet, getDisplayLink indeed generates the CADisplayLink in case it's missing (here)

Thanks for the contribution!

asafkorem commented 6 months ago

@tyronet-sportsbet the fix has released in Detox v20.18.4