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

It's possible to lease blocks from the pool after the pool has been disposed #1815

Closed
davidfowl opened this issue May 6, 2017 · 2 comments
Closed
Assignees

Comments

@davidfowl
Copy link
Member

When using connection adapters, it's possible to leak memory on shutdown if the adapted pipeline runs after the libuv thread is stopped. Generally though, this shouldn't happen if the shutdown is graceful.

@davidfowl
Copy link
Member Author

Here's what a typical trace looks like:

Hosting starting
Hosting started
Loaded hosting startup assembly Microsoft.AspNetCore.Server.Kestrel.FunctionalTests, Version=2.0.0.0, Culture=neutral, PublicKeyToken=adb9793829ddae60
Connection id "0HL4L3C6U9KGO" started.
Applying connection adapters
before Microsoft.AspNetCore.Server.Kestrel.FunctionalTests.ConnectionAdapterTests+AsyncConnectionAdapter.ApplyConnectionAdaptersAsync
Connection id "0HL4L3C6U9KGO" received FIN.
Connection id "0HL4L3C6U9KGO" disconnecting.
Connection id "0HL4L3C6U9KGO" stopped.
Connection id "0HL4L3C6U9KGO" sending FIN.
Hosting shutdown
Disposing everything (this runs after the libuv thread ends)
after Microsoft.AspNetCore.Server.Kestrel.FunctionalTests.ConnectionAdapterTests+AsyncConnectionAdapter.ApplyConnectionAdaptersAsync
Make the adapted pipeline
before adaptedPipeline.RunAsync()

davidfowl added a commit that referenced this issue May 7, 2017
- Building on top of the last refactoring of FrameConnection, this change aims to clean up
the communication between the Frame and FrameConnection by removing some concepts and
being consistent about the communication between Frame and FrameConnection with or without
connection adapters. Changes include:
- Removing ConnectionLifetimeControl, ISocketOutput, StreamSocketOutput
- Moving more initialization of the frame to FrameConnection after the pipes
are setup
- OutputProducer communicates cancellation via the IPipeWriter instead of the output's IPipeReader.
- Frame always communicates via the pipes and that communications flows through the layers to the transport.
This means that each 1/2 of the adapted pipeline handles closing the right side of the transport at the
right time, propagating exceptions as necessary.
- This is how the flow looks now:
            ->                        ->
[transport]     [connection adapters]     [frame]
            <-                        <-
- Transports need to handle a ConnectionAbortedException on the output as a signal to stop
writing and end the connection. This will no longer try to drain the output but will just stop
writing and end the response immediately.

#1815
@davidfowl
Copy link
Member Author

It's because it's possible to start the shutdown sequence before the frame is added to the list of connections.

davidfowl added a commit that referenced this issue May 9, 2017
* Refactoring and of FrameConnection and Frame
- Building on top of the last refactoring of FrameConnection, this change aims to clean up
the communication between the Frame and FrameConnection by removing some concepts and
being consistent about the communication between Frame and FrameConnection with or without
connection adapters. Changes include:
- Removing ConnectionLifetimeControl, ISocketOutput, StreamSocketOutput
- Moving more initialization of the frame to FrameConnection after the pipes
are setup
- OutputProducer communicates cancellation via the IPipeWriter instead of the output's IPipeReader.
- Frame always communicates via the pipes and that communications flows through the layers to the transport.
This means that each 1/2 of the adapted pipeline handles closing the right side of the transport at the
right time, propagating exceptions as necessary.
- This is how the flow looks now:
            ->                        ->
[transport]     [connection adapters]     [frame]
            <-                        <-
- Transports need to handle a ConnectionAbortedException on the output as a signal to stop
writing and end the connection. This will no longer try to drain the output but will just stop
writing and end the response immediately.
- Remove frame.Abort when cancellation on Write fails.
- Unify the connection shutdown logic
- Dispose 1/2 initialized connection adapters

#1815
@davidfowl davidfowl self-assigned this May 9, 2017
@davidfowl davidfowl added this to the 2.0.0-preview2 milestone May 9, 2017
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

1 participant