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

Resolve addresses when creating a new stream #1342

Merged
merged 2 commits into from
Mar 31, 2022

Conversation

gammazero
Copy link
Contributor

BasicHost.NewStream will try to establish a connection if one doesn't already exist. This will fail if the destination host's addresses have not yet been resolved. This PR resolves the destination host's addresses before creating the stream and possible new connection.

Fixes #1302

BasicHost.NewStream will try to establish a connection if one doesn't already exist.  This will fail if the hosts addresses have not yet been resolved.  This PR resolves the hosts addresses before creating the stream and possible new connection.

Fixes #1302
Copy link
Member

@Stebalien Stebalien left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will make a DNS request every time even if we already have a connection. Take a look at the code in RoutedHost.NewStream, we should do the same thing.

We should probably also change the implementation in RoutedHost to set "nodial" so we don't try to dial twice (well, three times...)

@marten-seemann marten-seemann merged commit e8d3df9 into master Mar 31, 2022
@gammazero gammazero deleted the fix/new-stream-resolve branch April 19, 2022 09:59
gammazero added a commit to ipni/storetheindex that referenced this pull request Jan 18, 2023
This code is not needed as the problem was fixed by this PR: libp2p/go-libp2p#1342
gammazero added a commit to ipni/storetheindex that referenced this pull request Jan 18, 2023
* Remove code to resolve address before gostream.Dial

This code is not needed as the problem was fixed by this PR: libp2p/go-libp2p#1342

* No longer necessary to explicitly close resource manager

It is not necessary to explicitly close a libp2p host's resource manager. The problem was fixed in this PR: libp2p/go-libp2p#1343
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.

BasicHost.NewStream doesn't resolve DNS addresses
3 participants