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

WeakRef support #501

Closed
ronag opened this issue Dec 29, 2020 · 5 comments · Fixed by #502
Closed

WeakRef support #501

ronag opened this issue Dec 29, 2020 · 5 comments · Fixed by #502
Labels
good first issue Good for newcomers Tests Changes related to the unit tests

Comments

@ronag
Copy link
Member

ronag commented Dec 29, 2020

We should test that a WeakRef:d Pool or Client does not get garbage collected while there is an active socket.

So basically if the pool/client has no pending request and idle/keepalive timeout has expired it should be garbage collected, otherwise not.

I think this already works but needs a regression test.

Client has a ref to Pool through onDrain listener and Socket has a ref to Client through [kClient].

I think sockets are GC roots so I believe they will never be GC:d while there is still an active connection.

@ronag ronag added good first issue Good for newcomers Tests Changes related to the unit tests labels Dec 29, 2020
@ronag
Copy link
Member Author

ronag commented Dec 29, 2020

@Ethan-Arrowood @dnlup Maybe? This can be fun if you want to get more familiar with weak refs.

@ronag ronag changed the title Add test for WeakRef WeakRef support Dec 29, 2020
@dnlup
Copy link
Contributor

dnlup commented Dec 29, 2020

To test if it has been garbage collected, we could use deref, right? As for keeping the socket active, is it sufficient to work just with the keepalive timeout, or is it better to send some data (in this case, I don't know how to do it without referencing the Client/Pool atm)? I can try to put something together.

@ronag
Copy link
Member Author

ronag commented Dec 29, 2020

I think you can use finalization registry as well to check for gc

@ronag
Copy link
Member Author

ronag commented Dec 29, 2020

I don't know how to do it without referencing the Client/Pool

I mean you can write something and then null the refs?

@dnlup
Copy link
Contributor

dnlup commented Dec 29, 2020

Yup, thanks for the pointers

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers Tests Changes related to the unit tests
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants