Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Provide offset to placeholder in apphost pack #3829

Open
nguerrera opened this issue Nov 4, 2019 · 1 comment
Open

Provide offset to placeholder in apphost pack #3829

nguerrera opened this issue Nov 4, 2019 · 1 comment
Labels
area-HostModel Microsoft.NET.HostModel issues
Milestone

Comments

@nguerrera
Copy link
Contributor

Today, the SDK does a search and replace in the apphost, to find the placeholder. However, the offset to the placeholder is fixed for any given apphost pack. It would be cleaner if this offset could be determined when the apphost template is built and described in a simple sidecar file.

The code that does the search fails on some filesystems today, due to MAP_SHARED not being implemented. We could implement the search a different way, but risk regressing perf for an operation that needn't be done on every end-user's build, and can be done once in our build.

cc @peterhuene @swaroop-sridhar @jeffschwMSFT

@nguerrera
Copy link
Contributor Author

The idea of providing the offset was raised in the initial PR that implemented search and replace: dotnet/sdk#978 (comment)

We were missing the additional data point that the memory mapped code would fail on some filesystems, though.

@swaroop-sridhar swaroop-sridhar self-assigned this Nov 4, 2019
@msftgits msftgits transferred this issue from dotnet/core-setup Jan 30, 2020
@msftgits msftgits added this to the 5.0 milestone Jan 30, 2020
swaroop-sridhar added a commit to swaroop-sridhar/runtime that referenced this issue Feb 18, 2020
This change attempts to fix a non-deterministic customer reported failure.

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, unless the failure is a knwon irrecoverable code
(listed a few error codes relevent to File IO).

This change may cause unnecessary retries on legitimate failures (about 50 seconds).
But such cases are rare, because the SDK supplies the apphost, and the HostModel itself creates the file to update.

Fixes dotnet#3832
swaroop-sridhar added a commit that referenced this issue Feb 19, 2020
This change attempts to fix a non-deterministic customer reported failure.

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 #3828 and #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, unless the failure is a knwon irrecoverable code (listed a few error codes relevent to File IO).

This change may cause unnecessary retries on legitimate failures (about 50 seconds).
But such cases are rare, because the SDK supplies the apphost, and the HostModel itself creates the file to update.

Fixes #3832
@maryamariyan maryamariyan added the untriaged New issue has not been triaged by the area owner label Feb 23, 2020
@jeffschwMSFT jeffschwMSFT removed the untriaged New issue has not been triaged by the area owner label Feb 24, 2020
@swaroop-sridhar swaroop-sridhar modified the milestones: 5.0.0, 6.0.0 Jun 26, 2020
@swaroop-sridhar swaroop-sridhar removed their assignment Sep 8, 2020
@vitek-karas vitek-karas modified the milestones: 6.0.0, Future Aug 10, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-HostModel Microsoft.NET.HostModel issues
Projects
None yet
Development

No branches or pull requests

6 participants