Skip to content
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

close channel when thread resouces are freed #91

Merged
merged 2 commits into from
Nov 30, 2017

Conversation

mtanami
Copy link

@mtanami mtanami commented Nov 29, 2017

quick and less risky solution for the zombies channel

@mtanami
Copy link
Author

mtanami commented Nov 29, 2017

Please review
@abrandwi
@ilayze
@yyellin

@ilayze
Copy link
Contributor

ilayze commented Nov 29, 2017

I don't understand how does it solves the problem? If a thread is being closed by the thread pool how do you know about it?

@ilayze
Copy link
Contributor

ilayze commented Nov 29, 2017

Oh I think I got it, you override the finalize...

@ilayze
Copy link
Contributor

ilayze commented Nov 29, 2017

Did you test it is actually cleans up the channel? I mean when the Thread is being shrink by the thread pool are you sure the ChannelWrapper ThreadLocal is being garbage collected?

@mtanami
Copy link
Author

mtanami commented Nov 29, 2017 via email

@ilayze
Copy link
Contributor

ilayze commented Nov 29, 2017

Ok, I would be glad to help with this test if you like. And if it really works it is the most elegant and most correct solution!

@ilayze
Copy link
Contributor

ilayze commented Nov 30, 2017

I test it on my environment and it works very well! merging

@ilayze ilayze merged commit 8a61093 into foundation-runtime:master Nov 30, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants