-
Notifications
You must be signed in to change notification settings - Fork 10.2k
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
[Reopen] Fix ANCM installer on ARM64 #59483
base: main
Are you sure you want to change the base?
Conversation
src/Servers/IIS/AspNetCoreModuleV2/AspNetCore/HandlerResolver.cpp
Outdated
Show resolved
Hide resolved
src/Servers/IIS/AspNetCoreModuleV2/AspNetCore/HandlerResolver.cpp
Outdated
Show resolved
Hide resolved
@halter73, can you confirm your local testing showed that this patch worked as expected? |
Thanks for reopening your PR @lextm! Your work has been very helpful.
Yes. I can verify that the patch worked as expected at least for the in-process handler when publishing apps targeting ARM64 (already worked), x64 and x86. More importantly, it didn't crash on app pool startup. I plan on verifying out-of-process scenarios as soon as we get a build. I was a little surprised to see the PR included the runtime change to call @BrennanConroy Can you also take a quick look to verify this makes sense? |
src/Installers/Windows/AspNetCoreModule-Setup/Forwarders/build.proj
Outdated
Show resolved
Hide resolved
Question 1: Can you explain exactly what is fixed by expanding the handler library search path to include the current dll folder?Let's review the actual folder structure to see why: Windows x64
In this case, if we use Windows ARM64Like I proposed in this pull request, the folder structure is changed as follows
If we keep using Question 2: Do I need to test out-of-process hosting to see the difference it makes?Yes, you need to test out-of-process hosting in order to see the difference |
/azp run |
Azure Pipelines successfully started running 3 pipeline(s). |
Win64="$(var.IsWin64)"> | ||
<File Id="AspNetCoreModuleV2Dll.forwarder" | ||
Name="aspnetcorev2.dll" | ||
Source="$(var.ArtifactsDir)\bin\AspNetCoreModuleForwarders\aspnetcorev2.dll" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I ran a non-PR build against this branch, so I could test the updated hosting bundle on an arm64 machine myself. Unfortunately, I got a build error due to this file missing. And there's a similar error for aspnetcorev2_outofprocess.dll
below.
##[error]src\Installers\Windows\AspNetCoreModule-Setup\ANCMIISExpressV2\ancm_iis_expressv2.wxs(171,0): error LGHT0103: (NETCORE_ENGINEERING_TELEMETRY=Build) The system cannot find the file 'D:\a_work\1\s\artifacts\bin\AspNetCoreModuleForwarders\aspnetcorev2.dll'.
https://dev.azure.com/dnceng-public/public/_build/results?buildId=908627&view=results
I started a new build at https://dev.azure.com/dnceng-public/public/_build/results?buildId=909937&view=results with the binlog enabled. I can look more into this tomorrow, but I posting this now in case @wtgodbe or @lextm has any insights that could save me time.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I left a comment in AncmV2.wixproj
to indicate how such dll files (pure forwarders) are generated in the workflow, so you might take a look there.
@@ -81,4 +81,8 @@ | |||
'$(PackageIconFullPath)' ^ | |||
'$(PackageLicenseExpression)' " /> | |||
</Target> | |||
|
|||
<Target Name="BeforeBuild" Condition="'$(Platform)' == 'arm64'"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@halter73 The forwarder dll files are generated by this target. You might review MSBuild binlog to see why it wasn't triggered when you built the artifacts. I guess it was caused by condition mismatch.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could it be that this BeforeBuild
target is simply missing from AncmIISExpressV2.wixproj
? I tried pushing to the lextm-47290
branch at [email protected]:lextudio/httpplatformhandlerv2.git
to update this PR and test it, but I got a permission error.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
En, I didn't add this target to AncmIISExpressV2.wixproj
because not necessary if building everything out. But like you found out, if just building a few projects (not the entire solution) then that missing target can be a problem and lead to missing files during ANCM IIS Express installer builds.
I just pushed a new commit as you suggested addressing this.
Looks like this PR hasn't been active for some time and the codebase could have been changed in the meantime. |
@@ -110,7 +110,7 @@ HandlerResolver::LoadRequestHandlerAssembly(const IHttpApplication &pApplication | |||
|
|||
LOG_INFOF(L"Loading request handler: '%ls'", handlerDllPath.c_str()); | |||
|
|||
hRequestHandlerDll = LoadLibrary(handlerDllPath.c_str()); | |||
hRequestHandlerDll = LoadLibraryEx(handlerDllPath.c_str(), nullptr, LOAD_LIBRARY_SEARCH_DEFAULT_DIRS | LOAD_LIBRARY_SEARCH_DLL_LOAD_DIR); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hRequestHandlerDll = LoadLibraryEx(handlerDllPath.c_str(), nullptr, LOAD_LIBRARY_SEARCH_DEFAULT_DIRS | LOAD_LIBRARY_SEARCH_DLL_LOAD_DIR); | |
hRequestHandlerDll = LoadLibrary(handlerDllPath.c_str()); |
Under
C:\Program Files\IIS\Asp.Net Core Module\V2\17.0.23030
aspnetcorev2_outofprocess.dll
(pure forwarder)aspnetcorev2_outofprocess_x64.dll
(x64 out-of-process dll)aspnetcorev2_outofprocess_arm64.dll
(ARM64 out-of-process dll)...
Yes, you need to test out-of-process hosting in order to see the difference
LoadLibraryEx
makes. WithLoadLibrary
you should get a file not found error, while withLoadLibraryEx
the bitness specific dll is properly loaded.
I've tried to reproduce the file not found error after reverting this change to HandlerResolver
, but I'm unable to.
I've tried various combinations of out-of-process publish targets including portable, win-arm64, win-x64 and win-x86 with and without setting both "Enable 32-Bit Applications" and "Enable Emulation on ARM64" on the app pool. In every case, it appears to load just aspnetcorev2_outofprocess.dll
and work just fine. It never loads aspnetcorev2_outofprocess_x64.dll
or aspnetcorev2_outofprocess_arm64.dll
after the revert, but it makes sense to me that for out-of-process scenarios, the handler and webapp bitness don't need to match.
DLLs loaded with LoadLibraryEx
change
DLLs loaded without LoadLibraryEx
change (reverted)
As you can see from these procmon screenshots of dlls loaded by w3wp, if I revert the LoadLibraryEx
change, only aspnetcorev2_outofprocess.dll
gets loaded and not aspnetcorev2_outofprocess_x64.dll
even when "Enable Emulation on ARM64" is set for the running app pool. Fortunately, this doesn't make a difference even for apps that are published targeting win-64.
You can try it yourself by grabbing the "Windows_HostingBundle" from https://dev.azure.com/dnceng-public/public/_build/results?buildId=925824&view=artifacts&pathAsName=false&type=publishedArtifacts
Am I missing any steps necessary to reproduce the file not found error?
If we cannot reproduce it, I think we should revert to the old LoadLibrary
that only searches the default dirs. It's usually best not to change more than we have to. I think this also means we can get rid of both aspnetcorev2_outofprocess_x64.dll
and aspnetcorev2_outofprocess_arm64.dll
.
Revised ASP.NET Core module installers for Windows ARM64
LoadLibraryEx
withLOAD_LIBRARY_SEARCH_DLL_LOAD_DIR
flag set, so that the pure forwarder works as desired.Fixes #47115.
Description
About the change in
HandlerResolver.cpp
To implement all proposed installer changes in #47115, then in the out-of-process folder three files present,
aspnetcorev2_outofprocess.dll
, an ARM64X pure forwarder.aspnetcorev2_outofprocess_arm64.dll
, the pure ARM64 handler.aspnetcorev2_outofprocess_x64.dll
, the x64 handler.Before this change,
LoadLibrary
only loads the pure forwarder, but then scans the default folders (the dll folder is not included) to load the actual handler which obviously fails.After this change,
LoadLibraryEx
can pick up the correct handler as the dll folder is added in the search folder list.About files in
Forwarders
folderSource code for the two ARM64X pure forwarders.
About changes in
ANCMV2
folderBefore this change, only ARM64 web apps can run on IIS.
After this change, the new file structure enables ARM64/x64/x86 side by side execution on IIS.
About changes in
ANCMIISEXpressV2
folderBefore this change, only ARM64 web apps can run on IIS Express.
After this change, the new file structure enables ARM64/x64/x86 side by side execution on IIS Express.