Skip to content

Add SetLastError to GdiPlus methods (#59096) - #60201

Merged
Anipik merged 2 commits into
dotnet:release/5.0from
safern:PortSetLastErrorGdip
Oct 13, 2021
Merged

Add SetLastError to GdiPlus methods (#59096)#60201
Anipik merged 2 commits into
dotnet:release/5.0from
safern:PortSetLastErrorGdip

Conversation

@safern

@safernsafern commented Oct 8, 2021

Copy link
Copy Markdown
Member

Backport of #59096 to release/5.0

Issue: #58741

Customer Impact

Regression from .NETFramework and .NETCore causing a unexpected ExternalException related to session transitions (sleep/wake, Remote Desktop, etc)

Testing

Manually verified in debugger. Unit tests.

Risk

Very low. Adds back SetLastError to only those PINvokes which require it.

@ericstj@safern
Add SetLastError to GdiPlus methods (dotnet#59096)
56deccf
System.Drawing uses the last Win32 error in CheckErrorStatus to try to
guess at why a GdiPlus draw method failed and ignore some failures.
This was broken in .NETCore when SetLastError was removed from PINvokes.
Bring back SetLastError for all PInvokes that use CheckErrorStatus.
@safernsafern added the Servicing-consider Issue for next servicing release review label Oct 8, 2021
@ghost

ghost commented Oct 8, 2021

Copy link
Copy Markdown

Tagging subscribers to this area: @safern, @tarekgh
See info in area-owners.md if you want to be subscribed.

Issue Details

Backport of #59096 to release/5.0

Issue: #58741

Customer Impact

Regression from .NETFramework and .NETCore causing a unexpected ExternalException related to session transitions (sleep/wake, Remote Desktop, etc)

Testing

Manually verified in debugger. Unit tests.

Risk

Very low. Adds back SetLastError to only those PINvokes which require it.

Author:safern
Assignees:-
Labels:

Servicing-consider, area-System.Drawing

Milestone:-

tarekgh
tarekgh approved these changes Oct 8, 2021
View reviewed changes

@tarekghtarekgh left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Just wondering, does GDIplus APIs set the last errors in all cases?

@safern

safern commented Oct 8, 2021

Copy link
Copy Markdown
MemberAuthor

does GDIplus APIs set the last errors in all cases?

@ericstj did you look for that info when doing the initial fix on main and 6.0?

@safernsafern added Servicing-approved Approved for servicing release and removed Servicing-consider Issue for next servicing release review labels Oct 11, 2021
@runfoapprunfoappBot mentioned this pull request Oct 11, 2021
Closed
@safern

safern commented Oct 12, 2021

Copy link
Copy Markdown
MemberAuthor

/azp run runtime

@azure-pipelines

azure-pipelinesBot commented Oct 12, 2021

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

@Anipik

Anipik commented Oct 13, 2021

Copy link
Copy Markdown
Contributor

failures are unrelated so merging this one.

@Anipik
Anipik merged commit 5c7b1c7 into dotnet:release/5.0Oct 13, 2021
@AnipikAnipik mentioned this pull request Oct 13, 2021
Merged
@safern
safern deleted the PortSetLastErrorGdip branch October 13, 2021 05:47
@ghostghost locked as resolved and limited conversation to collaborators Nov 12, 2021
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@stephentoubstephentoubstephentoub approved these changes
@tarekghtarekghtarekgh approved these changes
@JeremyKuhneJeremyKuhneAwaiting requested review from JeremyKuhne
@ericstjericstjAwaiting requested review from ericstj
+2 more reviewers
@danmoseleydanmoseleydanmoseley approved these changes
@AnipikAnipikAnipik approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

Servicing-approvedApproved for servicing release

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@safern@Anipik@stephentoub@danmoseley@tarekgh@ericstj