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 crash in PlayerCore.fileStarted, #3822 #4334

Merged
merged 1 commit into from
Apr 23, 2023
Merged

Fix crash in PlayerCore.fileStarted, #3822 #4334

merged 1 commit into from
Apr 23, 2023

Conversation

low-batt
Copy link
Contributor

@low-batt low-batt commented Apr 9, 2023

This commit will:

  • Add Atomic to PlaybackInfo.matchedSubs
  • Add a getMatchedSubs method to PlaybackInfo
  • Change AutoFileMatcher, PlayerCore and PlaylistViewController to hold a lock while accessing matchedSubs
  • Add Atomic to PlayerCore.backgroundQueueTicket
  • Change PlayerCore.fileStarted to hold a lock while incrementing backgroundQueueTicket
  • Add code to PlayerCore.stop that increments backgroundQueueTicket

These changes insure the mutable Swift dictionary matchedSubs is only accessed by one thread at a time. Accessing a mutable dictionary from multiple threads without such coordination can result in a crash as seen in issue #3822.

These changes also attempt to insure the background task that finds and loads subtitle files stops its work if playback is stopped before that process completes.


Description:

This commit will:
- Add Atomic to PlaybackInfo.matchedSubs
- Add a getMatchedSubs method to PlaybackInfo
- Change AutoFileMatcher, PlayerCore and PlaylistViewController to hold
  a lock while accessing matchedSubs
- Add Atomic to PlayerCore.backgroundQueueTicket
- Change PlayerCore.fileStarted to hold a lock while incrementing
  backgroundQueueTicket
- Add code to PlayerCore.stop that increments backgroundQueueTicket

These changes insure the mutable Swift dictionary matchedSubs is only
accessed by one thread at a time. Accessing a mutable dictionary from
multiple threads without such coordination can result in a crash as seen
in issue #3822.

These changes also attempt to insure the background task that finds and
loads subtitle files stops its work if playback is stopped before that
process completes.
@low-batt
Copy link
Contributor Author

low-batt commented Apr 9, 2023

Reviewers should question whether the code patterns used for locking in the commit can be improved as such locking will need to be applied to other properties.

@low-batt low-batt linked an issue Apr 9, 2023 that may be closed by this pull request
1 task
@uiryuu uiryuu requested a review from lhc70000 April 17, 2023 13:22
@uiryuu uiryuu mentioned this pull request Apr 23, 2023
23 tasks
@uiryuu uiryuu merged commit f28a7f2 into develop Apr 23, 2023
2 checks passed
@uiryuu uiryuu deleted the fix-3822 branch April 23, 2023 05:43
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.

Crash in objc_release called from PlayerCore.fileStarted
2 participants