New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Weak ThreadId still leaks threads #488

Closed
kazu-yamamoto opened this Issue Dec 15, 2015 · 6 comments

Comments

Projects
None yet
2 participants
@kazu-yamamoto
Contributor

kazu-yamamoto commented Dec 15, 2015

It is well known that threads are leaked if we hold their ThreadIds. That's why we are using Weak ThreadId in Warp. Even if we hold Weak ThreadIds, stacks of the threads are released. However, other data structures relating to the threads are not released. We can watch this by +RTS -hT.

I believe that this is a bug of GHC. Probably relating to: https://ghc.haskell.org/trac/ghc/ticket/10812

Since the timeout manager takes actions every 30 seconds, the data structures are leaked for 30 seconds maximum. This can be easily fixed to introduce IORef to onTimeout.

@kazu-yamamoto

This comment has been minimized.

Contributor

kazu-yamamoto commented Dec 15, 2015

@snoyberg In addition to introduce IORef to onTimeout, I would like to add a new API - killManager which actually kills the timeout manager immediately. This is very important for HTTP/2 since a timeout manager is launched for every HTTP/2 connection. After closing an HTTP/2 connection, the manager should go away immediately. For this, I would like to introduce reaperKill to Reaper.

I have already confirmed that this works well on my local branch. @snoyberg What do you think?

@snoyberg

This comment has been minimized.

Member

snoyberg commented Dec 15, 2015

I don't see any problem with such a change.

On Tue, Dec 15, 2015 at 1:11 PM, Kazu Yamamoto notifications@github.com
wrote:

@snoyberg https://github.com/snoyberg In addition to introduce IORef to
onTimeout, I would like to add a new API - killManager which actually
kills the timeout manager immediately. This is very important for HTTP/2
since a timeout manager is launched for every HTTP/2 connection. After
closing an HTTP/2 connection, the manager should go away immediately. For
this, I would like to introduce reaperKill to Reaper.

I have already confirmed that this works well on my local branch.
@snoyberg https://github.com/snoyberg What do you think?


Reply to this email directly or view it on GitHub
#488 (comment).

kazu-yamamoto added a commit that referenced this issue Dec 16, 2015

@kazu-yamamoto

This comment has been minimized.

Contributor

kazu-yamamoto commented Dec 16, 2015

Done in the merged branch above. We use ThreadId instead of Weak ThreadId and ensure that ThreadId is released quickly.

@kazu-yamamoto

This comment has been minimized.

Contributor

kazu-yamamoto commented Dec 16, 2015

@snoyberg I would like to release a new version of auto-update. Would you add me to the maintainer list?

@snoyberg

This comment has been minimized.

Member

snoyberg commented Dec 16, 2015

Added

On Wed, Dec 16, 2015 at 6:35 AM, Kazu Yamamoto notifications@github.com
wrote:

@snoyberg https://github.com/snoyberg I would like to release a new
version of auto-update. Would you add me to the maintainer list?


Reply to this email directly or view it on GitHub
#488 (comment).

@kazu-yamamoto

This comment has been minimized.

Contributor

kazu-yamamoto commented Dec 16, 2015

Thanks. Released.

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