Task.Delay in Dispose method of LoopbackListener in IdentityModel.OidcClient

Question

I'm using IdentityModel.OidcClient's SystemBrowser to request a token in a console app via browser. What I don't understand is, why there is an await Task.Delay(500) in the Dispose method of the LoopbackHttpListener. I see why you would do the Dispose via Task.Run, but not the delay.

Line of code in GitHub: https://github.com/IdentityModel/IdentityModel.OidcClient/blob/46d7b25cd71eb0be5f3c203c2525b1f357e408db/clients/ConsoleClientWithBrowser/SystemBrowser.cs#L131

    public LoopbackHttpListener(int port, string path = null)
    {
        path = path ?? String.Empty;
        if (path.StartsWith("/")) path = path.Substring(1);

        _url = $"http://127.0.0.1:{port}/{path}";

        _host = new WebHostBuilder()
            .UseKestrel()
            .UseUrls(_url)
            .Configure(Configure)
            .Build();
        _host.Start();
    }

    public void Dispose()
    {
        Task.Run(async () =>
        {
            await Task.Delay(500);
            _host.Dispose();
        });
    }

That's not a question that can be answered by anyone other than the maintainers. Did you check the `git blame` for that line? It says `hacky dispose of Kestrel`, committed 6 years ago. You should probably create a Github issue asking why that's there. After 6 years there are probably better ways to do this. IAsyncDisposable at least would remove the need for `Task.Run` — Panagiotis Kanavos, Aug 25 '22 at 09:44
As for why there's a delay, it's probably a really hacky way to wait for requests that are still running to complete. It would be better to call `WebHost.StartAsync` and `StopAsync` instead — Panagiotis Kanavos, Aug 25 '22 at 09:52
@PanagiotisKanavos Why don't you write full answer? There is your [commit](https://github.com/IdentityModel/IdentityModel.OidcClient/commit/1d7645605bae53fa6a0ee7c2764b5d6d45b2b18a), called "hacky dispose of Kestrel" — Nenad, Aug 25 '22 at 09:56
@Nenad because I don't know the answer. I can only guess. I don't know enough about OpenID Connect and why an endpoint like this is needed. I know there are complex workflows but never used them. The file names alone suggest this is a demo client which doesn't get frequently updated. The csproj targets .NET 6 directly, not even .NET Core 3.1 — Panagiotis Kanavos, Aug 25 '22 at 10:22

score 2 · Answer 1 · answered Aug 25 '22 at 11:09

I don't know why that code is used but I can guess. This could only be answered by the maintainers, assuming they still remember why this was written after 6 years.

Clicking on Blame shows that this specific line is 6 years old, tagged as hacky dispose of Kestrel. Recent commits have messages like Update test client or Update sample client. The csproj file targets .NET 6 directly. If this was library code it would target .NET Standard 2.1 or .NET Core 3.1 as well, which are still supported.

My guess is that this code is used to ensure any pending requests handled by the listener complete before it closes. Since this started as test code, some hacks were used and never fixed, because they didn't directly impact the test's or sample's core behavior.

I don't remember what WebHost or the Generic Host looked back in 2017 (and I'm too lazy to find out). The interfaces have changed a lot between .NET Standard 2.0, .NET Core 3.1 and .NET 6.

In .NET 6 though we can use IAsyncDisposable to allow asynchronous disposal. Inside DisposeAsync we can call WebHost.StopAsync to gracefully shut down the host before disposing it:

public class LoopbackHttpListener : IAsyncDisposable
{

...
    public async ValueTask DisposeAsync()
    {
        await _host.StopAsync(TimeSpan.FromMilliseconds(500));
        _host.Dispose();
    }

It's possible the LoopbackHttpListener can be replaced by Minimal APIs too.

var builder = WebApplication.CreateBuilder(args);
var app = builder.Build();

app.MapGet("/", (HttpRequest request) =>{
    _source.TrySetResult(request.QueryString);
    return "<h1>You can now return to the application.</h1>";
});

_task=app.RunAsync(_url);

The test code returns a fixed response on any HTTP GET call and exposes the request's query string through the _source TaskCompletionSource.

My thought was to use `IAsyncDisposable` as well. I just wasn't sure if there is more to the Delay, but you are right, the commit message suggests otherwise. — Kobidor, Aug 25 '22 at 11:34

Task.Delay in Dispose method of LoopbackListener in IdentityModel.OidcClient

1 Answers1