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

Use runtime property settings instead of environment variables for setting startup hook #9520

Closed
jkotalik opened this issue Apr 18, 2019 · 6 comments · Fixed by #10121
Closed
Assignees
Labels
accepted This issue has completed "acceptance" testing (including accessibility) area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions enhancement This issue represents an ask for new feature or an enhancement to an existing one feature-iis Includes: IIS, ANCM

Comments

@jkotalik
Copy link
Contributor

jkotalik commented Apr 18, 2019

Epic #8833

The startup hook today is set via environment variable right now (https://github.com/aspnet/AspNetCore/blob/master/src/Servers/IIS/AspNetCoreModuleV2/InProcessRequestHandler/inprocessapplication.cpp#L241). Core-setup will eventually be adding an api to set runtime properties like https://github.com/dotnet/core-setup/pull/5336/files#diff-15fec1485f5f10c590d5b840300283ceR233. We should use these instead.

@analogrelay
Copy link
Contributor

Blocked on dotnet/core-setup#5859

@analogrelay analogrelay added the enhancement This issue represents an ask for new feature or an enhancement to an existing one label May 7, 2019
@analogrelay
Copy link
Contributor

analogrelay commented May 7, 2019

This is now available in hostfxr 3.0.0-preview6-27707-04, when that lands in our repo (in the next day or so). Reminder (to myself) to check if it's arrived in the triage tomorrow

〉 dumpbin /exports "C:\Users\anurse\Desktop\hostfxr.dll"
Microsoft (R) COFF/PE Dumper Version 14.21.27619.1
Copyright (C) Microsoft Corporation.  All rights reserved.


Dump of file C:\Users\anurse\Desktop\hostfxr.dll

File Type: DLL

  Section contains the following exports for hostfxr.dll

    00000000 characteristics
    FFFFFFFF time date stamp
        0.00 version
           1 ordinal base
          15 number of functions
          15 number of names

    ordinal hint RVA      name

          1    0 00018F60 hostfxr_close
          2    1 00018FB0 hostfxr_get_available_sdks
          3    2 000192A0 hostfxr_get_native_search_directories
          4    3 00019410 hostfxr_get_runtime_delegate
          5    4 000194D0 hostfxr_get_runtime_properties
          6    5 00019600 hostfxr_get_runtime_property_value
          7    6 00019780 hostfxr_initialize_for_app
          8    7 00019900 hostfxr_initialize_for_runtime_config
          9    8 000199F0 hostfxr_main
         10    9 00019B00 hostfxr_main_startupinfo
         11    A 00019BF0 hostfxr_resolve_sdk
         12    B 00019E80 hostfxr_resolve_sdk2
         13    C 0001A130 hostfxr_run_app
         14    D 0001A180 hostfxr_set_error_writer
         15    E 0001A190 hostfxr_set_runtime_property_value <--- !!

@analogrelay
Copy link
Contributor

Looks like it's in our repo now:

〉 dumpbin /exports C:\Code\aspnet\AspNetCore\.dotnet\x64\host\fxr\3.0.0-preview6-27706-03\hostfxr.dll
Microsoft (R) COFF/PE Dumper Version 14.21.27619.1
Copyright (C) Microsoft Corporation.  All rights reserved.


Dump of file C:\Code\aspnet\AspNetCore\.dotnet\x64\host\fxr\3.0.0-preview6-27706-03\hostfxr.dll

File Type: DLL

  Section contains the following exports for hostfxr.dll

    00000000 characteristics
    FFFFFFFF time date stamp
        0.00 version
           1 ordinal base
          15 number of functions
          15 number of names

    ordinal hint RVA      name

          1    0 00018890 hostfxr_close
          2    1 000188E0 hostfxr_get_available_sdks
          3    2 00018BD0 hostfxr_get_native_search_directories
          4    3 00018D40 hostfxr_get_runtime_delegate
          5    4 00018E00 hostfxr_get_runtime_properties
          6    5 00018F30 hostfxr_get_runtime_property_value
          7    6 000190B0 hostfxr_initialize_for_app
          8    7 00019230 hostfxr_initialize_for_runtime_config
          9    8 00019320 hostfxr_main
         10    9 00019430 hostfxr_main_startupinfo
         11    A 00019520 hostfxr_resolve_sdk
         12    B 000197B0 hostfxr_resolve_sdk2
         13    C 00019A60 hostfxr_run_app
         14    D 00019AB0 hostfxr_set_error_writer
         15    E 00019AC0 hostfxr_set_runtime_property_value

  Summary

        4000 .data
        6000 .pdata
       25000 .rdata
        1000 .reloc
        1000 .rsrc
       5F000 .text

@analogrelay analogrelay added cost: XS and removed blocked The work on this issue is blocked due to some dependency labels May 8, 2019
@jkotalik jkotalik added cost: S and removed cost: XS labels May 8, 2019
@jkotalik
Copy link
Contributor Author

jkotalik commented May 8, 2019

This change will actually be a lot larger than I expected. It requires a whole new set of hostfxr apis to be used, and may expand into more issues. Defensively putting cost M on it.

@analogrelay analogrelay added the blocked The work on this issue is blocked due to some dependency label May 14, 2019
@analogrelay
Copy link
Contributor

@jkotalik jkotalik removed the Working label May 28, 2019
@jkotalik jkotalik added Working and removed blocked The work on this issue is blocked due to some dependency labels Jun 4, 2019
@jkotalik
Copy link
Contributor Author

jkotalik commented Jul 1, 2019

Acceptance checklist (check one item)

  • We decided not to take this fix.
  • The fix is tests-only.
  • The fix contains product changes (check all items below).
    • Relevant XML documentation comments for new public APIs are present.
    • Narrative docs (docs.microsoft.com) are updated. (check one item below)
      • The change requires a new article. An issue with an outline has been filed here: [ISSUE LINK]
      • The change requires a change to an existing article. A docs PR with these changes is linked here: [PR LINK]
      • The change requires no docs changes.
    • Verification has been completed. (check one item below)
      • Verified against ASP.NET Core App Version 3.0.0-preview7.19351.2

@jkotalik jkotalik added the accepted This issue has completed "acceptance" testing (including accessibility) label Jul 1, 2019
@ghost ghost locked as resolved and limited conversation to collaborators Dec 3, 2019
@amcasey amcasey added area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions and removed area-runtime labels Jun 2, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
accepted This issue has completed "acceptance" testing (including accessibility) area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions enhancement This issue represents an ask for new feature or an enhancement to an existing one feature-iis Includes: IIS, ANCM
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants