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

Add a hint to the port conflict panic #1484

Closed
teor2345 opened this issue Dec 9, 2020 · 2 comments · Fixed by #1535
Closed

Add a hint to the port conflict panic #1484

teor2345 opened this issue Dec 9, 2020 · 2 comments · Fixed by #1535
Assignees
Labels
A-rust Area: Updates to Rust code C-bug Category: This is a bug
Milestone

Comments

@teor2345
Copy link
Contributor

teor2345 commented Dec 9, 2020

Analysis

The port conflict panic message is hard to understand, we should add a hint about changing the default port.

This change should be made with #1509, because they need similar hints.

Version

v1.0.0-alpha.0

Platform

Unknown

Description

it is syncing, after changing the default listen address

if you have zcashd occupying 8233 it just panics with zebra_network::peer::connection: e=buffered service failed: buffered service failed: Address already in use (os error 98)

@teor2345 teor2345 added C-bug Category: This is a bug A-rust Area: Updates to Rust code S-needs-triage Status: A bug report needs triage labels Dec 9, 2020
@teor2345 teor2345 added this to the First Alpha Release milestone Dec 9, 2020
@oxarbitrage
Copy link
Contributor

We need to find out if the port we are going to bind is already in use. The sooner we do it is better as this is a fatal condition. Right now, the failure we see is deeply inside zebra-network. I think we should check and exit long before, somewhere around https://github.com/ZcashFoundation/zebra/blob/main/zebrad/src/commands/start.rs#L46

Next, how to do the check. I am unsure if there is an easy way with what we currently have. One option will be to add a new dependency: https://docs.rs/port_scanner/0.1.5/port_scanner/fn.local_port_available.html

Looking for advice. Thanks!

@teor2345
Copy link
Contributor Author

We need to find out if the port we are going to bind is already in use. The sooner we do it is better as this is a fatal condition. Right now, the failure we see is deeply inside zebra-network. I think we should check and exit long before, somewhere around https://github.com/ZcashFoundation/zebra/blob/main/zebrad/src/commands/start.rs#L46

Next, how to do the check. I am unsure if there is an easy way with what we currently have. One option will be to add a new dependency: https://docs.rs/port_scanner/0.1.5/port_scanner/fn.local_port_available.html

Looking for advice. Thanks!

Unfortunately, there's no reliable cross-platform way to check if a port is already in use. That's because different operating systems have different rules for port conflicts. And some OSes allow programs to open ports that never conflict.

We should also avoid trying to open the port early, closing it, then opening it for real. Because that can lead to a temporary failure due to the TCP retransmit interval.

So the best way to handle this error is to try to use the port in zebra-network, and provide a helpful message if using the port fails.

If we wanted that message to happen earlier, we could try to open the listener port early in zebrad's start function, and then pass the open port to zebra-network. But I don't think that's a good design, because the port opening code belongs in zebra-network:
https://github.com/ZcashFoundation/zebra/blob/main/zebrad/src/commands/start.rs#L43

@mpguerra mpguerra removed the S-needs-triage Status: A bug report needs triage label Dec 14, 2020
@teor2345 teor2345 removed this from the v1.0.0-alpha.1 milestone Jan 11, 2021
@mpguerra mpguerra modified the milestones: 2021 Sprint 1, 2021 Sprint 2 Jan 25, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-rust Area: Updates to Rust code C-bug Category: This is a bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants