Skip to content
This repository has been archived by the owner on Jan 23, 2023. It is now read-only.

[release/3.1] HostModel: Retry ResourceUpdate on Win32 error #9012

Merged
merged 1 commit into from
Mar 25, 2020

Conversation

swaroop-sridhar
Copy link

Issue

dotnet/runtime#3832

Customer Scenario

Building a WinExe with resources fails non-deterministically

Several customers have observed failure during resource update when the HostModel updates the AppHost (to transfer resources from the managed app).
The failure is not detereminisitc, not reproducible on our machines, and depends on specific computers/software running.
This indicates interference by other software while the HostWriter is updating the AppHost.

Problem

The current implementation retries the resource update if an update because the device or drive is locked (say by an antivurus) HRESULT 0x21 and 0x6C. However, the failures reported have errors 0x5 (Access violation) and 0x6# (Open failed). Windows/Defender team said that file-locking with these error-codes is not expected.
However, different AVs work differently about examining files.

Solution

We believe that the correct fix for this issue is to complete:

However the above is a fairly large change for servicing .net core 3.1.

So, this change implements a simpler fix intended for servicing 3.1 branch: Always retry the resource-update on Win32 error. While this may cause unnecessary retries on legitimate failures (ex: file missing) such cases are rare, because the SDK supplies the apphost, and the HostModel itself creates the file to update.

Risk

Low

Master Branch PR

dotnet/runtime#32347

dotnet/runtime#3832

Building a WinExe with resources fails non-deterministically

Several customers have observed failure during resource update when the HostModel updates the AppHost (to transfer resources from the managed app).
The failure is not detereminisitc, not reproducible on our machines, and depends on specific computers/software running.
This indicates interference by other software while the HostWriter is updating the AppHost.

The current implementation retries the resource update if an update because the device or drive is locked (say by an antivurus) HRESULT 0x21 and 0x6C. However, the failures reported have errors 0x5 (Access violation) and 0x6# (Open failed). Windows/Defender team said that file-locking with these error-codes is not expected.
However, different AVs work differently about examining files.

We believe that the correct fix for this issue is to complete:

* To implement dotnet#3828 and dotnet#3829
* Ship AppHost with an extension/permissions not indicating an executable.

However the above is a fairly large change for servicing .net core 3.1.

So, this change implements a simpler fix intended for servicing 3.1 branch: Always retry the resource-update on Win32 error. While this may cause unnecessary retries on legitimate failures (ex: file missing) such cases are rare, because the SDK supplies the apphost, and the HostModel itself creates the file to update.

Low

dotnet/runtime#32347
@swaroop-sridhar swaroop-sridhar requested a review from wli3 February 28, 2020 01:24
Comment on lines +55 to +57
case 0x00000008: // ERROR_NOT_ENOUGH_MEMORY
case 0x0000000B: // ERROR_BAD_FORMAT
case 0x0000000E: // ERROR_OUTOFMEMORY

Choose a reason for hiding this comment

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

Love the fact that there's an error code for both "not enough memory" and "out of memory" :)

Copy link
Member

Choose a reason for hiding this comment

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

Possibly analogous to our distinction between InsufficientMemoryException and OutOfMemoryException.

Copy link

@lpereira lpereira left a comment

Choose a reason for hiding this comment

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

I'm not familiar enough with Win32 to properly review this, but patch seems correct.

@swaroop-sridhar swaroop-sridhar added area-HostModel Microsoft.NET.HostModel issues Servicing-consider Issue for next servicing release review labels Feb 28, 2020
@jeffschwMSFT jeffschwMSFT added this to the 3.1.x milestone Mar 3, 2020
@jeffschwMSFT jeffschwMSFT added Servicing-approved Approved for servicing release and removed Servicing-consider Issue for next servicing release review labels Mar 4, 2020
@jeffschwMSFT jeffschwMSFT modified the milestones: 3.1.x, 3.1.4 Mar 4, 2020
@jeffschwMSFT
Copy link
Member

Approved in tactics

@avifatal
Copy link

Hi,
How do I update my dockers to avoid this problem?
Thanks

@Anipik Anipik merged commit 6a52bfa into dotnet:release/3.1 Mar 25, 2020
@swaroop-sridhar
Copy link
Author

@avifatal the fix will be in the next servicing release of the .net core 3.1 SDK.
Once this is available, you can use the new SDK to avoid the problem.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-HostModel Microsoft.NET.HostModel issues Servicing-approved Approved for servicing release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants