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

Get travis-ci working #7

Closed
lawrencelomax opened this issue Sep 11, 2015 · 1 comment
Closed

Get travis-ci working #7

lawrencelomax opened this issue Sep 11, 2015 · 1 comment

Comments

@lawrencelomax
Copy link
Contributor

Now that Travis has been enabled for this repo, we should fix the .travis.yml and get builds and tests running.

@lawrencelomax
Copy link
Contributor Author

Done

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
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant