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

Expire listening addresses if a listener is closed #1482

Closed
tomaka opened this issue Mar 3, 2020 · 2 comments · Fixed by #1485
Closed

Expire listening addresses if a listener is closed #1482

tomaka opened this issue Mar 3, 2020 · 2 comments · Fixed by #1485

Comments

@tomaka
Copy link
Member

tomaka commented Mar 3, 2020

In listeners.rs, if a listener is closed, we emit a ListenersEvent::Closed event, but we don't mark the addresses the listener was listening on as "expired".

The ListenersEvent::Closed event should contain a list of addresses, and then we should appropriately call NetworkBehaviour::inject_expired_listen_addr and emit SwarmEvent::ExpiredListenAddr.

@alxndrmkd
Copy link

Hello, I would like to take this one, if you don't mind.

@tcharding
Copy link
Contributor

tcharding commented Mar 9, 2020

This Draft PR is an attempt at resolving this. It is draft because I have some rustfmt dramas.

tcharding added a commit to tcharding/rust-libp2p that referenced this issue Mar 9, 2020
Add an addresses field to the ListenersEvent and the ListenerClosed to
hold the addresses of a listener that has just closed. When we return a
ListenerClosed network event loop over the addresses and call
inject_expired_listen_address on each one.

Fixes: libp2p#1482
tomaka added a commit that referenced this issue Mar 23, 2020
* Add addresses field for closing listeners

Add an addresses field to the ListenersEvent and the ListenerClosed to
hold the addresses of a listener that has just closed. When we return a
ListenerClosed network event loop over the addresses and call
inject_expired_listen_address on each one.

Fixes: #1482

* Use Vec instead of SmallVec

In order to not expose a third party dependency in our API use a `Vec`
type for the addresses list instead of a `SmallVec`.

* Do not clone for ListenersEvent::Closed

We would like to avoid clones where possible for efficiency reasons.
When returning a `ListenersEvent::Closed` we are already consuming the
listener (by way of a pin projection).  We can therefore use a consuming
iterator instead of cloning.

Use `drain(..).collect()` instead of clone to consume the addresses when
returning a `ListenersEvent::Closed`.

* Expire addresses before listener

The listener and its addresses technically expire at the same time, but
since here we have to pick an order, it makes more sense that the
addresses expire first.

Co-authored-by: Pierre Krieger <pierre.krieger1708@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants