-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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 incorrectly terminated background task #21254
Conversation
Generated by 🚫 dangerJS |
It's unlikely it's the reason for terminations, but better safe than sorry. |
if let task = self?.bgTask, task != .invalid { | ||
DDLogInfo("BackgroundTask: executing expirationHandler for bgTask = \(task.rawValue)") | ||
app.endBackgroundTask(task) | ||
self?.bgTask = .invalid |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
❓ I agree with the removal of DispatchQueue.main.async
, but do you have an idea why it was added in the first place? Maybe it was added to fix some bug. But even in that case, the ideal solution shouldn't be DispatchQueue.main.async
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tried to check the history, but this code has been moved around a bit.
Based on the comment ("synchronize on the main thread"), this async is unnecessary because the expiration handler is already called on the main thread:
The system calls the handler synchronously on the main thread, blocking the app’s suspension momentarily.
From https://developer.apple.com/documentation/uikit/uiapplication/1623031-beginbackgroundtaskwithexpiratio
📲 You can test the changes from this Pull Request in WordPress Alpha by scanning the QR code below to install the corresponding build.
|
📲 You can test the changes from this Pull Request in Jetpack Alpha by scanning the QR code below to install the corresponding build.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
The app currently prints the following error message when the background task expiration handler is called:
The reason is that the app doesn't end the task immediately on termination. The expiration handler is already called on the main queue, so there is no need to dispatch it.
This may be one of the reasons of WatchdogTermination events.
To test:
Regression Notes
PR submission checklist:
RELEASE-NOTES.txt
if necessary.UI Changes testing checklist: