Skip to content
This repository was archived by the owner on Dec 18, 2018. It is now read-only.

Close connections as quickly and gracefully as possible on shutdown#623

Merged
halter73 merged 13 commits into
devfrom
shalter/stop-it
Feb 18, 2016
Merged

Close connections as quickly and gracefully as possible on shutdown#623
halter73 merged 13 commits into
devfrom
shalter/stop-it

Conversation

@halter73
Copy link
Copy Markdown
Member

AbortAwaiting();

// Return all blocks
var block = _head;
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.

Is this safe without locks?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This is only called after the socket is closed so there should not be any more incoming data. The AbortAwaiting call kills any consumers. (If all else fails, the finalizer still returns the block in production, but it would crash the process in debug builds)

@benaadams benaadams mentioned this pull request Feb 12, 2016
8 tasks
@benaadams
Copy link
Copy Markdown
Contributor

Does this provide a deterministic disposal point for the connection? (rather than any finalization)

@benaadams
Copy link
Copy Markdown
Contributor

Wanted to do a pooled SocketOutput - but bit complicated atm

}

public void Stop(TimeSpan timeout)
{
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Add

#if !DEBUG
    if (_thread != null)
    {
        _thread.IsBackground = true;
    }
#end

- Make the time given for requests to complete gracefully configurable.
- Complete all async reads so calling code can re-check whether to stop
  request processing and exit if in between requests.
- Don't wait for a FIN from the client since some browsers (e.g. IE & Chrome)
  will take a long time to send one.
- Ensure all ConnectionFilters complete before the memory pool is disposed.
- Ensure blocks get returned even when a ConnectionFilter produces a failed read
- Even when safe handles are disposed explicitly, ReleaseHandle is sometimes
  called on another thread which breaks uv_close.
- Ensure we close the UvAsyncHandle the uv loop so that the second call
  to uv_run always completes without a timeout/Thread.Abort.
- Re-enable some tests. Add skip conditions for those that aren't passing.
- This means connections become untracked sooner than before and not all blocks will
  necessarily be returned.
- The assertion in the MemoryPoolBlock2 finalizer was weakened because FilteredStreamAdapter
  will continue to use blocks after libuv stops tracking the associated connection.
- Make 100% sure we don't accept new connections after we dispose the listen socket by using a flag.
- Add a (currently unused) AllowStop method to KestrelThread. This is meant to be called from
  listeners when we stop accepting new connections, but needs investigation to prevent flakiness.
- Maybe things are better now with graceful shutdown *crosses fingers*
@halter73
Copy link
Copy Markdown
Member Author

@benaadams the "deterministic disposal point for the connection" would be Connection.OnSocketClosed. Right now, the only time I'm aware of that it might not be called is during ungraceful shutdown if KestrelThread.OnStopRude disposes the socket.

I don't think there is a need for it currently, but if you want OnSocketClosed to always be called, you could simply cast the UvHandle like ConnectionManager does in OnStopRude, and call OnSocketClosed there.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants