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 after all nodes dissappear #33

Closed
Kester-Broatch opened this issue Jul 13, 2023 · 2 comments · Fixed by #34
Closed

Close channel after all nodes dissappear #33

Kester-Broatch opened this issue Jul 13, 2023 · 2 comments · Fixed by #34
Labels
bug Something isn't working

Comments

@Kester-Broatch
Copy link
Contributor

Hi there,

I noticed when you disconnect a UDP server (ip = 0.0.0.0) endpoint and we get a node disappeared message for the device(s) at that endpoint the channel never closes.

This has the effect that mavp2p will continue sending messages to that channel indefinitely, i confirmed this with wireshark. In our case it keeps sending UDP messages out into the void over cellular, so its a quite expensive issue! 😅

So it would be good to close a UDP server channel when all known nodes on it disappear.

What are your thoughts?

Thanks, Kester

@aler9 aler9 added the bug Something isn't working label Aug 7, 2023
aler9 added a commit to bluenviron/gomavlib that referenced this issue Aug 7, 2023
…environ/mavp2p#33) (#61)

* replace pkg/udplistener with pion/transport/v2/udp

* add node.IdleTimeout and close idle connections after a timeout (bluenviron/mavp2p#33)
aler9 added a commit that referenced this issue Aug 7, 2023
aler9 added a commit that referenced this issue Aug 7, 2023
@aler9 aler9 closed this as completed in #34 Aug 7, 2023
aler9 added a commit that referenced this issue Aug 7, 2023
@aler9
Copy link
Member

aler9 commented Aug 7, 2023

Hello, thanks for reporting the issue. I checked and the library didn't provide any mechanism to close idle channels, so i did the following:

@Kester-Broatch
Copy link
Contributor Author

Hi, thanks for implementing this fix! I will implement and test this fix in the next few weeks and get back to you :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants