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

fix(ios): handle async player access in text track selection #4293

Merged

Conversation

tarikfp
Copy link
Contributor

@tarikfp tarikfp commented Nov 18, 2024

Fix Potential Race Condition in Video Player Text Track Selection

Description

Potentially fixed a crash that happens when text tracks get selected while the player is being destroyed. Super hard to reproduce but shows up in crash logs. Basically the player exists when we start the async operation but might be gone by the time we try to use it.

Changes

  • Added weak self in async block
  • Added guard check for player instance
  • Made sure we have strong reference to player during async operation

Steps to Maybe Reproduce

Really hard to get this consistently, but here's what might trigger it:

  1. Switch videos really fast
  2. Change text tracks while video is loading
  3. Background the app during text track changes

Type of Change

  • Bug fix (non-breaking change that fixes an issue)
  • New feature (non-breaking change that adds functionality)
  • Breaking change (fix or feature that would break existing functionality)

Screenshots

ios-rctvideo-crash

Checklist

  • Code follows project style guidelines
  • Edge cases tested where possible
  • No new warnings introduced

@tarikfp tarikfp changed the title fix(ios): add null check to setSelectedTextTrack for player instance fix(ios): handle async player access in text track selection Nov 18, 2024
Copy link
Member

@KrzysztofMoch KrzysztofMoch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, Thanks for PR!

@KrzysztofMoch KrzysztofMoch merged commit daaac97 into TheWidlarzGroup:master Nov 22, 2024
6 checks passed
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

Successfully merging this pull request may close these issues.

2 participants