Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

fix(ios): message sent to deallocated instance #1482

Merged
merged 2 commits into from
Feb 14, 2019

Conversation

n1ru4l
Copy link
Contributor

@n1ru4l n1ru4l commented Feb 14, 2019

Provide an example of how to test the change

I had some EXC_BAD_ACCESS crashes in my app without an useful stack trace. After enabling zombie mode I was able to get the following log:

[RCTVideo retain]: message sent to deallocated instance 0x10c530890

I checked the code and eventually figured out that [_player removeObserver:self forKeyPath:externalPlaybackActive context: nil]; is not called upon dealloc.

I am not 100% sure this results in the crash. However after adding that line to dealloc I was not able to reproduce this issue with the simulator or my testing device. I will also send out a new testflight in order to verify that this is the case.

The crash seems to occur when you are quickly mounting and unmount video components e.g. through quick navigation.

@n1ru4l
Copy link
Contributor Author

n1ru4l commented Feb 14, 2019

Okay testflight users cannot reproduce this issue anymore.

@n1ru4l n1ru4l merged commit d8a2a9e into master Feb 14, 2019
@n1ru4l n1ru4l deleted the fix-message-deallocated-instance branch February 14, 2019 15:34
AnteWall pushed a commit to sfstudios/react-native-video that referenced this pull request Mar 1, 2019
* fix(ios): message sent to deallocated instance

* chore: update changelog
beauner69 pushed a commit to beauner69/react-native-video that referenced this pull request Oct 10, 2019
* fix(ios): message sent to deallocated instance

* chore: update changelog

(rebased from commit d8a2a9e)
# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant