Retrying on FBSimulatorInteraction #9
Comments
|
We now have this on However this should probably exist on |
|
Added by #21 |
plu
added a commit
to plu/FBSimulatorControl
that referenced
this issue
Feb 26, 2017
plu
added a commit
to plu/FBSimulatorControl
that referenced
this issue
Feb 26, 2017
facebook-github-bot
pushed a commit
that referenced
this issue
Nov 22, 2017
Summary: This code-path has possibly bit-rotted inside `SimulatorKit`, since it's pretty easy to cause a crash with the current implementation. Running with zombies enabled will occasionally yield the following, immediately after the owner of the `SimDisplayVideoWriter` has deallocated: ``` fbsimctl[58503:12175268] *** -[SimVideoMP4File release]: message sent to deallocated instance 0x104ba2c30 fbsimctl[58503:12175284] *** -[SimVideoMP4File completionHandler]: message sent to deallocated instance 0x104ba2c30 ```` Consequently causing a segfault on release. I believe that the reason for this may be down to the fact that the `completionHandler` is called *multiple times* which is likely driven by `dispatch_io` completion handlers. This suggests to me that the `SimVideoFile` is not weakly-referenced in a callback, but can become a dangling pointer. In the deallocation of `SimVideoFile`: ``` frame #0: 0x000000010560b110 SimulatorKit`-[SimVideoFile dealloc] frame #1: 0x000000010561b31d SimulatorKit`-[SimDisplayVideoWriter .cxx_destruct] + 43 frame #2: 0x00007fffcd38fbb4 libobjc.A.dylib`object_cxxDestructFromClass(objc_object*, objc_class*) + 127 frame #3: 0x00007fffcd3885f6 libobjc.A.dylib`objc_destructInstance + 92 frame #4: 0x00007fffb86000c6 CoreFoundation`-[NSObject(NSObject) __dealloc_zombie] + 150 frame #5: 0x00000001056190c5 SimulatorKit`-[SimDisplayVideoWriter dealloc] + 104 frame #6: 0x0000000100457f3c FBSimulatorControl`-[FBVideoEncoderSimulatorKit .cxx_destruct](self=0x00000001054210b0, _cmd=<no value available>) at FBVideoEncoderSimulatorKit.m:35 frame #7: 0x00007fffcd38fbb4 libobjc.A.dylib`object_cxxDestructFromClass(objc_object*, objc_class*) + 127 frame #8: 0x00007fffcd3885f6 libobjc.A.dylib`objc_destructInstance + 92 frame #9: 0x00007fffb86000c6 CoreFoundation`-[NSObject(NSObject) __dealloc_zombie] + 150 frame #10: 0x0000000100456e94 FBSimulatorControl`-[FBVideoEncoderSimulatorKit dealloc](self=0x00000001054210b0, _cmd="dealloc") at FBVideoEncoderSimulatorKit.m:89 frame #11: 0x00007fffcd3893bd libobjc.A.dylib`(anonymous namespace)::AutoreleasePoolPage::pop(void*) + 781 frame #12: 0x0000000100344e2d libdispatch.dylib`_dispatch_root_queue_drain + 1728 frame #13: 0x000000010034471d libdispatch.dylib`_dispatch_worker_thread3 + 114 frame #14: 0x00000001003b97da libsystem_pthread.dylib`_pthread_wqthread + 1299 frame #15: 0x00000001003b92b5 libsystem_pthread.dylib`start_wqthread + 13 ``` However, this sperlunking is likely indicative of why `simctl` has now changed to using `-[SimDisplayVideoWriter videoWriterForDispatchIO: fileType: completionQueue: completionHandler:]`. The implementation of the URL-based constructors is to wrap the construction of the `dispatch_io` constructor. Another interesting aspect is that the `dealloc` of `SimVideoFile` itself stimulates the `completionHandler` being called. Over-retaining the `SimVideoFile` so that it is not dealloced will cause the callbacka to *never* be called. During this, I discovered that the `completionHandler` was missing a parameter, which is a `nullable NSError *`, inline with how the other `completionHandler`s work in `CoreSimulator`. Upon cancellation, `ECANCELED` is wrapped in the `NSError *`, so we can handle this case as well. This may be slightly more reliable than the prior implementation, if there are further issues, we should migrate to using `dispatch_io` from above. Reviewed By: antiarchit Differential Revision: D6395394 fbshipit-source-id: 3077efdd471dd0265c44addfe84c0f8a1941ceba
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
id<FBSimulatorInteraction>can be called multiple times, some actions are flaky on certain OS versions (launching apps on XC7 GM). The API is chain able already so shouldn't be that big of an issue.The text was updated successfully, but these errors were encountered: