mirror of
https://github.com/hickory-dns/hickory-dns.git
synced 2026-04-25 03:05:51 +03:00
[GH-ISSUE #905] "proto error" was returned from lookup_ip #561
Labels
No labels
blocked
breaking-change
bug
bug:critical
bug:tests
cleanup
compliance
compliance
compliance
crate:all
crate:client
crate:native-tls
crate:proto
crate:recursor
crate:resolver
crate:resolver
crate:rustls
crate:server
crate:util
dependencies
docs
duplicate
easy
easy
enhance
enhance
enhance
feature:dns-over-https
feature:dns-over-quic
feature:dns-over-tls
feature:dnsssec
feature:global_lb
feature:mdns
feature:tsig
features:edns
has workaround
ops
perf
platform:WASM
platform:android
platform:fuchsia
platform:linux
platform:macos
platform:windows
pull-request
question
test
tools
tools
trust
unclear
wontfix
No milestone
No project
No assignees
1 participant
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference
starred/hickory-dns#561
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "%!s()"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Originally created by @zonyitoo on GitHub (Oct 28, 2019).
Original GitHub issue: https://github.com/hickory-dns/hickory-dns/issues/905
Describe the bug
I was using
AsyncResolverwithread_system_conf(). I have got "proto error" when resolving any addresses. Some useful logs are here:To Reproduce
Steps to reproduce the behavior:
Create a
AsyncResolverwithread_system_confCall
lookup_ipwith any addressesExpected behavior
lookup_ipshould return valid IPs instead of errors.System:
Version:
Crate: resolver
Version: master on github
@bluejekyll commented on GitHub (Oct 28, 2019):
Do you have the code for how tokio itself is being instantiated?
@bluejekyll commented on GitHub (Oct 28, 2019):
In some of the async/await changes, I think some lazy's were removed, this could be an issue in some cases. Possibly cause the issue your seeing, though I don't see that in any of the tests, which is why I'm wondering how Tokio is being instantiated in your examples.
@zonyitoo commented on GitHub (Oct 29, 2019):
I am using
#[tokio::main].@zonyitoo commented on GitHub (Oct 29, 2019):
This is how I instantiated tokio: https://github.com/shadowsocks/shadowsocks-rust/blob/feature-migrate-async-await/src/bin/server.rs#L25 .
In
run_serverfunction,AsyncResolveris instanciated inContext::new: https://github.com/shadowsocks/shadowsocks-rust/blob/feature-migrate-async-await/src/relay/server.rs#L18 .Context::new: https://github.com/shadowsocks/shadowsocks-rust/blob/feature-migrate-async-await/src/context.rs#L19 .create_resolver: https://github.com/shadowsocks/shadowsocks-rust/blob/feature-migrate-async-await/src/relay/dns_resolver.rs#L14@bluejekyll commented on GitHub (Oct 29, 2019):
Ok, using this comparison: https://github.com/bluejekyll/trust-dns/compare/4858edd3e74434e21bfa8c1a04b5a2d0459967da...master
My guess is that some of the lazy's that were removed in that patch are causing issues.
update: I looked through both the changes to resolver code and proto code to see if there was a lazy removed that would cause issues, and I don't see it. So I'm not sure that lazy is the issue here.
@bluejekyll commented on GitHub (Oct 29, 2019):
@zonyitoo if you have time, do you think you could throw together a test case that we can reproduce this problem with and we could use that as the basis for finding this issue and making sure it doesn’t happen again?
@zonyitoo commented on GitHub (Oct 29, 2019):
Minimun test case:
Cargo.tomldependencies:Rust version
Build and run with result (with all deps versions):
I can reproduce it everytime on my macbook.
@bluejekyll commented on GitHub (Oct 29, 2019):
Ok, thanks. I'll pull this into the repo and use as a new test case to make sure we're compatible with the usage of
#[tokio::main], my theory is that something is not as lazy as it should be during construction, but not sure why it's different based on the way tokio is instantiated.I might have time to look at this tonight, but can't guarantee it.
@zonyitoo commented on GitHub (Oct 29, 2019):
I don't think
#[tokio::main]is the one to be blamed. Here is another test case withRuntime:I can get exactly the same result:
@zonyitoo commented on GitHub (Oct 29, 2019):
This is exactly the test in async_resolver/mod.rs.
Not surprised ...
@bluejekyll commented on GitHub (Oct 29, 2019):
I wasn't implying it's at fault, just that it's exposing an issue in the library that wasn't there before.
What's confusing here is that this test passes in CI without issue, and on mac (also locally for me as well): https://travis-ci.org/bluejekyll/trust-dns/jobs/604008906#L827 (edit: had link to wrong line before)
I wonder if we're masking some other error?
@bluejekyll commented on GitHub (Oct 30, 2019):
Ok, here's why the tests are passing, change your dependency on tokio to
tokio = "0.2.0-alpha", and your example passes.So something about tokio master is different enough to break trust-dns.
@bluejekyll commented on GitHub (Oct 30, 2019):
Ok, see feedback on linked issue. After looking at the cargo tree output, I think it's clear that there are multiple dependency versions being used at once from tokio. Please use
0.2.0-alphafrom tokio, and most likely, it might be best to use the latest 0.18.0-alpha for trust-dns as well.@zonyitoo commented on GitHub (Oct 30, 2019):
Well, it works after changing deps of
tokio.@zonyitoo commented on GitHub (Oct 30, 2019):
Closing now.
@bluejekyll commented on GitHub (Oct 30, 2019):
If you want to use master, you’ll need do something to tell all the trust-dns libraries to also use master, it is possible: https://doc.rust-lang.org/cargo/reference/specifying-dependencies.html#overriding-dependencies
@zonyitoo commented on GitHub (Oct 30, 2019):
That's very helpful!! Thanks!
@zonyitoo commented on GitHub (Oct 30, 2019):
But it seems that trust-dns cannot build with tokio's master
@zonyitoo commented on GitHub (Oct 30, 2019):
These two errors could be fixed by replacing
tokio_netwithtokio::net.@bluejekyll commented on GitHub (Oct 30, 2019):
We don’t want to directly depend on Tokio, as that pulls in too many dependencies for the proto library. Did you override the version dependency for all of the Tokio child libraries?
@zonyitoo commented on GitHub (Oct 30, 2019):
I don't think that will work. Some of the child libraries have been merged completely into
tokio, such astokio-process,tokio-signal,tokio-io,tokio-net,tokio-executor...@cg31 commented on GitHub (Nov 19, 2019):
sub crate tokio-net was removed in tokio trunk:
github.com/tokio-rs/tokio@227533d456 (diff-35f6180221)@bluejekyll commented on GitHub (Nov 19, 2019):
Thanks for this note. I’m going to ping the Tokio dev channel to get some guidance.
@zonyitoo commented on GitHub (Nov 26, 2019):
Remind: tokio v0.2.0 is released.
@kpcyrd commented on GitHub (Mar 18, 2020):
I think I ran into the same issue after upgrading from osx 10.12 to 10.13. I'm on trust-dns 0.17.0 and trust-dns-proto 0.8.0.
Removing the fe80::/10 address from /etc/resolv.conf fixed this. I assume this error might be related to ipv6.
@bluejekyll commented on GitHub (Mar 18, 2020):
Any chance you can upgrade to 0.19?
@zonyitoo commented on GitHub (Mar 18, 2020):
Already upgraded. But found another warnings:
But errors in this issue is not showing again.