Skip to content

[release/6.0] Check BackgroundService.ExecuteTask for null - #60177

Merged
Anipik merged 2 commits into
release/6.0from
backport/pr-60139-to-release/6.0
Oct 8, 2021
Merged

[release/6.0] Check BackgroundService.ExecuteTask for null#60177
Anipik merged 2 commits into
release/6.0from
backport/pr-60139-to-release/6.0

Conversation

@github-actions

@github-actionsgithub-actionsBot commented Oct 8, 2021

Copy link
Copy Markdown
Contributor

Backport of #60139 to release/6.0

/cc @eerhardt

Customer Impact

.NET 6.0 introduced an undocumented breaking change. IHostedService startup is now special-cased for BackgroundService, where the Host takes a direct dependency on BackgroundService.ExecuteTask, which can be null in some cases that previously worked. This was reported by a 1st party using 6.0 rc1.

The fix is a simple null check to avoid a NullReferenceException.

Testing

Added a new automated unit test for this scenario to ensure it works.

Risk

Very low.

In some scenarios, derived classes might not have called base.StartAsync, and ExecuteTask will still be null. Ensure we don't fail in those cases.
Fix#60131
@eerhardteerhardt added the Servicing-consider Issue for next servicing release review label Oct 8, 2021

@danmoseleydanmoseley 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.

a faithful port :)

@AnipikAnipik added Servicing-approved Approved for servicing release and removed Servicing-consider Issue for next servicing release review labels Oct 8, 2021
@Anipik
Anipik merged commit abeae90 into release/6.0Oct 8, 2021
@Anipik
Anipik deleted the backport/pr-60139-to-release/6.0 branch October 8, 2021 19:09
@ghostghost locked as resolved and limited conversation to collaborators Nov 8, 2021
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area-Extensions-HostingServicing-approvedApproved for servicing release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@danmoseley@eerhardt@Anipik