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 Core Data concurrency issues in AccountService #20394

Merged
merged 3 commits into from
Mar 24, 2023

Conversation

crazytonyli
Copy link
Contributor

I noticed some Core Data concurrency issues on accessing WPAccount from a wrong thread.

I don't see a huge risk associated with these changes. The updateDefaultBlogIfNeeded function change is only a signature change, the code is still executed the way it used to be. The real logic of setupAppExtensionsWithDefaultAccount is now forced to run in the main thread, which is where it used to run too.

Test instructions

A couple of features to verify:

  • Log in and log out of the app using a WP.com account.
  • "Save as Draft" share extension (i.e. from Safari).

Regression Notes

  1. Potential unintended areas of impact
    None.

  2. What I did to test those areas of impact (or what existing automated tests I relied on)
    What's described in the "Test instructions"

  3. What automated tests I added (or what prevented me from doing so)
    None.

PR submission checklist:

  • I have completed the Regression Notes.
  • I have considered adding unit tests for my changes.
  • I have considered adding accessibility improvements for my changes.
  • I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary.

@crazytonyli crazytonyli added the Core Data Issues related to Core Data label Mar 24, 2023
@crazytonyli crazytonyli added this to the 22.1 milestone Mar 24, 2023
@crazytonyli crazytonyli requested a review from mokagio March 24, 2023 00:06
@crazytonyli crazytonyli self-assigned this Mar 24, 2023
@wpmobilebot
Copy link
Contributor

wpmobilebot commented Mar 24, 2023

Jetpack Alpha📲 You can test the changes from this Pull Request in Jetpack Alpha by scanning the QR code below to install the corresponding build.
App NameJetpack Alpha Jetpack Alpha
ConfigurationRelease-Alpha
Build Numberpr20394-b1a6ae8
Version22.0
Bundle IDcom.jetpack.alpha
Commitb1a6ae8
App Center Buildjetpack-installable-builds #4338
Automatticians: You can use our internal self-serve MC tool to give yourself access to App Center if needed.

@wpmobilebot
Copy link
Contributor

wpmobilebot commented Mar 24, 2023

WordPress Alpha📲 You can test the changes from this Pull Request in WordPress Alpha by scanning the QR code below to install the corresponding build.
App NameWordPress Alpha WordPress Alpha
ConfigurationRelease-Alpha
Build Numberpr20394-b1a6ae8
Version22.0
Bundle IDorg.wordpress.alpha
Commitb1a6ae8
App Center BuildWPiOS - One-Offs #5312
Automatticians: You can use our internal self-serve MC tool to give yourself access to App Center if needed.

@crazytonyli crazytonyli force-pushed the account-service-core-data-concurrency-issues branch from da24705 to e80dd11 Compare March 24, 2023 01:54
@crazytonyli crazytonyli force-pushed the account-service-core-data-concurrency-issues branch from e80dd11 to aaa4ce6 Compare March 24, 2023 01:54
@crazytonyli crazytonyli enabled auto-merge March 24, 2023 06:54
@crazytonyli crazytonyli merged commit bc6dceb into trunk Mar 24, 2023
@crazytonyli crazytonyli deleted the account-service-core-data-concurrency-issues branch March 24, 2023 07:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Core Data Issues related to Core Data
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants