Skip to content

Start and dispose the generic application host in ImageBuilder#2134

Open
Copilot wants to merge 4 commits into
mainfrom
copilot/fix-image-builder-host-management
Open

Start and dispose the generic application host in ImageBuilder#2134
Copilot wants to merge 4 commits into
mainfrom
copilot/fix-image-builder-host-management

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Jun 1, 2026

ImageBuilder built a Microsoft.Extensions.Hosting host only to expose its IServiceProvider, discarding the IHost. The container was never started, stopped, or disposed, so DI-owned disposable singletons (e.g. AddMemoryCache()) were never released deterministically.

Changes

  • ImageBuilder.cs — Retain the built host: Lazy<IServiceProvider> becomes Lazy<IHost>, exposed via a new internal AppHost. Services now derives from AppHost.Services; the public Commands API is unchanged.
  • Program.cs — Run command execution inside an explicit host lifetime: StartAsync() before resolving/invoking commands, StopAsync() on success, and dispose the host in finally so it is released on both success and failure. DisposeAsync() is used when the host supports IAsyncDisposable, falling back to Dispose().
IHost host = ImageBuilder.AppHost;
try
{
    await host.StartAsync();
    // ... build root command, invoke ...
    await host.StopAsync();
    return exitCode;
}
finally
{
    if (host is IAsyncDisposable asyncDisposableHost)
        await asyncDisposableHost.DisposeAsync();
    else
        host.Dispose();
}

Note

IHost only declares IDisposable, so the IAsyncDisposable cast is required to dispose asynchronously when the concrete host supports it — await host.DisposeAsync() does not compile against the interface.

Comment thread src/ImageBuilder/Program.cs Fixed
Copilot AI changed the title [WIP] Fix ImageBuilder to start and manage application host Start and dispose the generic application host in ImageBuilder Jun 1, 2026
Copilot AI requested a review from lbussell June 1, 2026 18:24
Replace the static `Lazy<IHost>` on `ImageBuilder` with a `CreateAppHost()`
factory so the composition root (`Program`) unambiguously builds, owns, and
disposes the host. Resolve commands directly via `GetServices<ICommand>()`
and drop the static `Services`/`Commands` accessors.

`StandaloneLoggerFactory` now owns a settable `ILoggerFactory` (defaulting to
`NullLoggerFactory.Instance`) that `Program` wires from the host, instead of
reaching into a static service provider. Convert `McrTagsMetadataGenerator`'s
type-load-time static logger field to a property so it no longer captures the
default factory before `Program` configures logging.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@lbussell lbussell marked this pull request as ready for review June 4, 2026 17:02
@lbussell lbussell requested a review from a team as a code owner June 4, 2026 17:02
Copy link
Copy Markdown
Member

@mthalman mthalman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also consider updating the PR description to reflect the evolved approach.

var parseResult = rootCliCommand.Parse(args);
int exitCode = await parseResult.InvokeAsync();

await host.StopAsync();
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't StopyAsync be executed even if an exception occurs?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ImageBuilder does not start the generic application host

3 participants