Skip to content

Use derived context for WeeklyRoundup background task - #17642

Merged
dvdchr merged 2 commits into
release/18.8from
fix/weekly-roundup-background-crash
Dec 8, 2021
Merged

Use derived context for WeeklyRoundup background task#17642
dvdchr merged 2 commits into
release/18.8from
fix/weekly-roundup-background-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.8 ❄️ milestone Dec 8, 2021
@dvdchr dvdchr self-assigned this Dec 8, 2021
@peril-wordpress-mobile

peril-wordpress-mobile Bot commented Dec 8, 2021

Copy link
Copy Markdown

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

@peril-wordpress-mobile

peril-wordpress-mobile Bot commented Dec 8, 2021

Copy link
Copy Markdown

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

@diegoreymendez diegoreymendez left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The failing test is due to an unrelated issue with a timeout that's too aggressive.

@dvdchr
dvdchr merged commit 5861993 into release/18.8 Dec 8, 2021
@dvdchr
dvdchr deleted the fix/weekly-roundup-background-crash branch December 8, 2021 16:26
@mokagio

mokagio commented Dec 8, 2021

Copy link
Copy Markdown
Contributor

@dvdchr this has been bundled as part of 18.8 beta 4 (18.8.0.4), which is currently building in CI.

Thanks for your work 🙌

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.

3 participants