Skip to content

Use derived context for WeeklyRoundup background task - #17640

Closed
dvdchr wants to merge 1 commit into
developfrom
fix/weekly-roundup-bg-crash
Closed

Use derived context for WeeklyRoundup background task#17640
dvdchr wants to merge 1 commit into
developfrom
fix/weekly-roundup-bg-crash

Conversation

@dvdchr

@dvdchr dvdchr commented Dec 8, 2021

Copy link
Copy Markdown
Contributor

Fixes #17639

This attempts to fix a crash due to the managed object context being accessed from the wrong thread. The WeeklyRoundupDataProvider is now initialized with a derived context so all of its operations won't be running on the main context and hopefully prevents overlap with any operations running in the main queue.

To test:

Refer to the steps described in #17066 and #17116, and make sure everything works.

Regression Notes

  1. Potential unintended areas of impact
    The Weekly Roundup feature might not work as expected.

  2. What I did to test those areas of impact (or what existing automated tests I relied on)
    Manually tested the scenarios described in Local Weekly Roundup #17066 and Implement weekly roundup toggle #17116.

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

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.

@dvdchr dvdchr added this to the 18.9 milestone Dec 8, 2021
@dvdchr
dvdchr requested a review from diegoreymendez December 8, 2021 09:15
@dvdchr dvdchr self-assigned this Dec 8, 2021
@peril-wordpress-mobile

Copy link
Copy Markdown

You can trigger an installable build for these changes by visiting CircleCI here.

@dvdchr dvdchr added Notifications and removed Stats labels Dec 8, 2021
@mokagio

mokagio commented Dec 8, 2021

Copy link
Copy Markdown
Contributor

@dvdchr how do you feel about targeting the release branch for this?

The metrics for the crash look encouraging in that the impact is limited, but, because of the December release pause, 18.9 is almost a month away from reaching the users.

Screen Shot 2021-12-08 at 10 28 33 am

@dvdchr

dvdchr commented Dec 8, 2021

Copy link
Copy Markdown
Contributor Author

how do you feel about targeting the release branch for this?

The metrics for the crash look encouraging in that the impact is limited, but, because of the December release pause, 18.9 is almost a month away from reaching the users.

@mokagio Yeah, that makes sense! As long as @diegoreymendez feels confident with the fix. Thank you for the heads up! I'll create a new PR based on release/18.8, cherry-picking the commit. 🙂

@peril-wordpress-mobile

Copy link
Copy Markdown

You can trigger optional UI/connected tests for these changes by visiting CircleCI here.

@dvdchr dvdchr removed this from the 18.9 milestone Dec 8, 2021
@dvdchr

dvdchr commented Dec 8, 2021

Copy link
Copy Markdown
Contributor Author

Closing in favor of #17642.

@dvdchr dvdchr closed this Dec 8, 2021
@dvdchr
dvdchr deleted the fix/weekly-roundup-bg-crash branch December 8, 2021 09:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

NSGenericException: Collection __NSCFSet was mutated while being enumerated

2 participants