-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
Use cross-platform code to update apphost win32 resources, open apphost only once for writing #3828
Comments
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
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
Hmm.. Any idea when this will be fixed? Now forced to build everything on Windows in a GitHub Action because of this. |
@agocke @vitek-karas Ping on this, any chance we can fix this for .NET 8? Now that we support Here's a recent discussion of this: dotnet/docs#34197 |
Same here. We're hitting this issue on GitHub Actions Ubuntu runners as all our workflows are tailored to Linux. This is not a minor inconvenience, indeed. |
Unfortunately, this is approaching the cut line for .NET 8. We may not have the resources to complete it in this release. But I will try to triage and see if we can get it in. |
It would really be a shame if this misses 8.0, we've been waiting so long for this, this issue itself is about 4 years old. |
We have been waiting for this more than 3 years now, ever since we changed from Mono to dotnet. Mono had the functionality. |
@mikkeljohnsen This is news to me -- if the code is easily available in Mono then it might be as simple as porting it over. We would certainly take a PR. |
Thanks a lot to @anatawa12 for taking this on - #89303 has been merged and will be in .NET 8. |
can you explain me what it does, i think the apphost is a c++ exe file copied and renamed and the dll is shared, why can't for win32 |
My understanding is this. AppHost for Windows GUI should have resources for app icon and else. So, .NET SDK copies resources from dll to AppHost exe. My pull-request adds cross-platform (managed only) code to add resources to AppHost exe instead of Win32 API invocation. |
NETSDK1074: The application host executable will not be customized because adding resources requires that the build be performed on Windows. See: dotnet/runtime#3828
NETSDK1074: The application host executable will not be customized because adding resources requires that the build be performed on Windows. See: dotnet/runtime#3828
NETSDK1074: The application host executable will not be customized because adding resources requires that the build be performed on Windows. See: dotnet/runtime#3828
NETSDK1074: The application host executable will not be customized because adding resources requires that the build be performed on Windows. See: dotnet/runtime#3828
NETSDK1074: The application host executable will not be customized because adding resources requires that the build be performed on Windows. See: dotnet/runtime#3828
NETSDK1074: The application host executable will not be customized because adding resources requires that the build be performed on Windows. See: dotnet/runtime#3828
NETSDK1074: The application host executable will not be customized because adding resources requires that the build be performed on Windows. See: dotnet/runtime#3828
NETSDK1074: The application host executable will not be customized because adding resources requires that the build be performed on Windows. See: dotnet/runtime#3828
NETSDK1074: The application host executable will not be customized because adding resources requires that the build be performed on Windows. See: dotnet/runtime#3828
On dotnet/core-setup#6831, it was noted that there now exists some portable code somewhere to update win32 resources, but we are still updating using Win32 API.
The downsides of using the win32 API are:
Prevents cross-compilation of apphost. You have to be on a windows build machine to build a windows apphost executable. Even a Windows Nano server is not up to the task.
Prevents something like Anti-Virus, sneaking in between different opens of the apphost. We currently open it once to patch in the app name over the placeholder, and again to patch the win32 resources. This should be done with a single open.
Various attempts have been made to get around the anti-virus (or anti-virus-like) issues with retries, but we are seeing that is insufficient with multiple users still running into issues regularly.
See https://github.com/dotnet/cli/issues/11650#issuecomment-549478350
cc @swaroop-sridhar @jeffschwMSFT @peterhuene @davidwrighton
The text was updated successfully, but these errors were encountered: