Skip to content
This repository has been archived by the owner on Mar 19, 2019. It is now read-only.

IApplicationShutdown.RequestShutdown is not honored on WebListener #71

Closed
Praburaj opened this issue Sep 26, 2014 · 8 comments
Closed
Assignees
Milestone

Comments

@Praburaj
Copy link
Contributor

Send a request to the application. It is expected to serve the outstanding requests and die. This works as expected on Helios. On weblistener this event is not being honored.

On Kestrel the event seems to be honored, but the outstanding request is not served. The host process simply gets killed. I'm filing a separate bug for kestrel in kestrel repo.

public void Configure(IApplicationBuilder app)
 {
app.UseServices();
app.Run(async context =>
                {
                    Console.WriteLine("Received shutdown request...");
                    var appShutdownToken = context.ApplicationServices.GetService<IApplicationShutdown>();
                    appShutdownToken.RequestShutdown();

                    try
                    {
                        if (!appShutdownToken.ShutdownRequested.IsCancellationRequested)
                        {
                            await Task.Delay(10 * 1000, appShutdownToken.ShutdownRequested);
                        }
                    }
                    catch (Exception exception)
                    {
                        Console.WriteLine(exception);
                    }

                    if (appShutdownToken.ShutdownRequested.IsCancellationRequested)
                    {
                        await context.Response.WriteAsync("Shutting down gracefully");
                    }
                    else
                    {
                        await context.Response.WriteAsync("Shutting down token not fired");
                    }
                });          
  }
@Tratcher
Copy link
Member

I think the problem is that RequestShutdown() blocks and waits for the requests to drain, but you're inside of a request so it deadlocks. It works fine if I offload the call to a backround Task.

            app.UseRequestServices();
            app.Use((context, next) =>
            {
                if (context.Request.Path == new PathString("/listen"))
                {
                    Console.WriteLine("Registering for shutdown notification...");
                    var appShutdownToken = context.ApplicationServices.GetService<IApplicationShutdown>();
                    appShutdownToken.ShutdownRequested.Register(() => Console.WriteLine("Shutdown notification fired."));
                    return context.Response.WriteAsync("Listening.");                    
                }
                return next();
            });
            app.Use((context, next) =>
            {
                if (context.Request.Path == new PathString("/shutdown"))
                {
                    Console.WriteLine("Received shutdown request...");
                    var appShutdownToken = context.ApplicationServices.GetService<IApplicationShutdown>();
                    Task.Run(() =>
                    {
                        appShutdownToken.RequestShutdown();
                        Console.WriteLine("Shutdown called.");
                    });                    
                    return context.Response.WriteAsync("Shutdown started.");
                }
                return next();
            });
            app.Use(async (context, next) =>
            {
                if (context.Request.Path == new PathString("/long"))
                {
                    Console.WriteLine("Starting long request");
                    var appShutdownToken = context.ApplicationServices.GetService<IApplicationShutdown>();

                    try
                    {
                        while (!appShutdownToken.ShutdownRequested.IsCancellationRequested)
                        {
                            await context.Response.WriteAsync("Waiting for shutdown.");
                            await Task.Delay(1000);
                        }
                        await context.Response.WriteAsync("Shutdown initiated.");
                        return;
                    }
                    catch (Exception exception)
                    {
                        Console.WriteLine(exception);
                    }
                }
                await next();
            });
            app.Run(async context =>
            {
                await context.Response.WriteAsync("<html><body>");
                await context.Response.WriteAsync("<a href=\"/listen\">listen</a><br>");
                await context.Response.WriteAsync("<a href=\"/long\">long</a><br>");
                await context.Response.WriteAsync("<a href=\"/shutdown\">shutdown</a><br>");
                await context.Response.WriteAsync("</body></html>");
            });

@Tratcher
Copy link
Member

Confirmed. RequestShutdown() synchronously triggers the ShutdownRequested CT, which Hosting uses to call Dispose on the server, which blocks while requests drain.

Helios calls RecycleProcess, or just Recycle. @GrabYourPitchforks I assume this does not block waiting for responses to drain?

Kestrel doesn't drain requests, it just starts closing sockets.

@davidfowl I think the IApplicationShutdown contract needs clarification. Should RequestShutdown() block until all registered callbacks complete (e.g. until requests are drained)? I'd say no, I think it should trigger the CT on a background thread and return immediately. If so, then the fix here is in Microsoft.Framework.Runtime.ApplicationShutdown.RequestShutdown().

@davidfowl
Copy link
Member

Interesting, I think you're right @Tratcher. @lodejard can you remember what we said in terms of waiting on subscribers to be called back?

@lodejard
Copy link

Yep, but lets talk in person. It's not something I can type on a phone

@Tratcher
Copy link
Member

DNX/Hosting needs to signal the event on a new thread as to not block the caller of RequestShutdown.

@Tratcher
Copy link
Member

TODO: Move to Hosting or DNX.

@Tratcher Tratcher modified the milestones: 1.0.0-beta6, 1.0.0-beta5 Jun 29, 2015
@Tratcher Tratcher self-assigned this Jun 29, 2015
@Tratcher
Copy link
Member

TODO: Verify repro. This may not block anymore since Hosting shutdown was refactored for ctl+c.

@Tratcher
Copy link
Member

Tratcher commented Jul 1, 2015

No repro, the Hosting shutdown logic has already changed and removed this deadlock.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

5 participants