From bddf7932badbbba4e962ea3995143b45c28d54aa Mon Sep 17 00:00:00 2001 From: Swaroop Sridhar Date: Thu, 27 Feb 2020 16:39:46 -0800 Subject: [PATCH] [release/3.1] HostModel: Retry ResourceUpdate on Win32 error https://github.com/dotnet/runtime/issues/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 #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. 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 https://github.com/dotnet/runtime/pull/32347 --- .../AppHost/RetryUtil.cs | 41 +++++++++++++++---- 1 file changed, 34 insertions(+), 7 deletions(-) diff --git a/src/managed/Microsoft.NET.HostModel/AppHost/RetryUtil.cs b/src/managed/Microsoft.NET.HostModel/AppHost/RetryUtil.cs index ef64d1e512..cbd4ccc2c9 100644 --- a/src/managed/Microsoft.NET.HostModel/AppHost/RetryUtil.cs +++ b/src/managed/Microsoft.NET.HostModel/AppHost/RetryUtil.cs @@ -41,16 +41,43 @@ public static void RetryOnIOError(Action func) public static void RetryOnWin32Error(Action func) { - bool IsWin32FileLockError(int hresult) + bool IsKnownIrrecoverableError(int hresult) { // Error codes are defined in winerror.h - const int ErrorLockViolation = 33; - const int ErrorDriveLocked = 108; - // The error code is stored in the lowest 16 bits of the HResult - int errorCode = hresult & 0xffff; - return errorCode == ErrorLockViolation || errorCode == ErrorDriveLocked; + switch (hresult & 0xffff) + { + case 0x00000001: // ERROR_INVALID_FUNCTION + case 0x00000002: // ERROR_FILE_NOT_FOUND + case 0x00000003: // ERROR_PATH_NOT_FOUND + case 0x00000006: // ERROR_INVALID_HANDLE + case 0x00000008: // ERROR_NOT_ENOUGH_MEMORY + case 0x0000000B: // ERROR_BAD_FORMAT + case 0x0000000E: // ERROR_OUTOFMEMORY + case 0x0000000F: // ERROR_INVALID_DRIVE + case 0x00000012: // ERROR_NO_MORE_FILES + case 0x00000035: // ERROR_BAD_NETPATH + case 0x00000057: // ERROR_INVALID_PARAMETER + case 0x00000071: // ERROR_NO_MORE_SEARCH_HANDLES + case 0x00000072: // ERROR_INVALID_TARGET_HANDLE + case 0x00000078: // ERROR_CALL_NOT_IMPLEMENTED + case 0x0000007B: // ERROR_INVALID_NAME + case 0x0000007C: // ERROR_INVALID_LEVEL + case 0x0000007D: // ERROR_NO_VOLUME_LABEL + case 0x0000009A: // ERROR_LABEL_TOO_LONG + case 0x000000A0: // ERROR_BAD_ARGUMENTS + case 0x000000A1: // ERROR_BAD_PATHNAME + case 0x000000CE: // ERROR_FILENAME_EXCED_RANGE + case 0x000000DF: // ERROR_FILE_TOO_LARGE + case 0x000003ED: // ERROR_UNRECOGNIZED_VOLUME + case 0x000003EE: // ERROR_FILE_INVALID + case 0x00000651: // ERROR_DEVICE_REMOVED + return true; + + default: + return false; + } } for (int i = 1; i <= NumberOfRetries; i++) @@ -61,7 +88,7 @@ bool IsWin32FileLockError(int hresult) break; } catch (HResultException hrex) - when (i < NumberOfRetries && IsWin32FileLockError(hrex.Win32HResult)) + when (i < NumberOfRetries && !IsKnownIrrecoverableError(hrex.Win32HResult)) { Thread.Sleep(NumMilliSecondsToWait); }