Defer search for dev cert until build bind time #48056
Conversation
...so that it can occur after IConfiguration is read. (In particular, so that it occurs during AddressBinder.BindAsync which happens strictly after KestrelConfigurationLoader.Load.) This is important in Docker scenarios since the Docker tools use IConfiguration to tell us where the dev cert directory is mounted.# with '#' will be ignored, and an empty message aborts the commit. Was dotnet#46296 Fixes dotnet#45801
amcasey
requested review from
Tratcher,
halter73,
BrennanConroy,
JamesNK and
mgravell
as code owners
ghost
added
the
amcasey
added
the
blocked
| var middleware = new HttpsConnectionMiddleware(next, httpsOptions, loggerFactory, metrics); | ||
| // Evaluate the HttpsConnectionAdapterOptions, now that the configuration, if any, has been loaded | ||
| var httpsOptions = lazyHttpsOptions.Value; | ||
| var loggerFactory = listenOptions.KestrelServerOptions.ApplicationServices.GetRequiredService<ILoggerFactory>(); |
There was a problem hiding this comment.
Will these service lookups here have a performance implication?
There was a problem hiding this comment.
It could be moved outside of the lambda so it's not happening per connection.
There was a problem hiding this comment.
+1. They were outside the lambda before.
There was a problem hiding this comment.
For my own edification, this is a squeeze-out-every-drop type optimization, right? We're saving a lookup in a (small) dictionary and a null check?
There was a problem hiding this comment.
It's a no-need-to-do-duplicate-work cleanup. We don't expect it to have a huge per impact, but there's no reason to do that extra work here, it's no cleaner.
There was a problem hiding this comment.
Done. Would it be worthwhile to make the variables Lazy so the lookups aren't wasted in HTTP/3-only scenarios?
|
Will try eagerly loading configuration instead. |
So that configuration-based certs will be considered, if necessary. Largely salvaged from dotnet#48056 Builds on dotnet#48137 Fixes dotnet#45801
amcasey
added
area-networking
Defer search for dev cert until build bind time
Description
...so that it can occur after IConfiguration is read. (In particular, so that it occurs during AddressBinder.BindAsync which happens strictly after KestrelConfigurationLoader.Load.) This is important in Docker scenarios since the Docker tools use IConfiguration to tell us where the dev cert directory is mounted.# with '#' will be ignored, and an empty message aborts the commit.
Was #46296
Fixes #45801