mirror of
https://github.com/hickory-dns/hickory-dns.git
synced 2026-04-25 03:05:51 +03:00
[GH-ISSUE #456] LookUp Future is not Sendable between threads #196
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#196
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 (May 11, 2018).
Original GitHub issue: https://github.com/hickory-dns/hickory-dns/issues/456
I am using the latest
tokiowithResolverFuture.tokiorequiresFutures to beSendable.Since the
ResolverFuture::client_cacheandResolverFuture::with_cacheare both private, so I put theResolverFuturein a global static variable like this:and then initialize
GLOBAL_DNS_RESOLVERin the future oftokio::run.That works fine except when I wanted to resolve a name:
resolver.lookup_ipreturns aLookupIpFuture, which is not sendable between threads:this is BAD!
I don't want to create a
ResolverFutureevery time when I want to resolve an address, because I wanted to make a good use of the DnsCache inside of it.So what can I do?
@bluejekyll commented on GitHub (May 11, 2018):
I think this is related to #429.
There is definitely some cleanup that needs to happen here. There may be a potential fix with #430, where by making the Lookup futures more lazy we can make them sendable. I was going to start looking into some solutions to this soon, but if you have ideas I'm open to hearing them.
@bluejekyll commented on GitHub (May 12, 2018):
FYI, I'm starting to look at options here. I'll see about creating an example that would do exactly what you're trying.
@zonyitoo commented on GitHub (May 13, 2018):
Glad to hear that!!
@bluejekyll commented on GitHub (May 13, 2018):
I've been planning to improve Send+Sync throughout the libraries for a while. B/C my usage doesn't end up needing this, it's been easy to accidentally drop Send/Sync.
I've made decent progress on Send, Sync will be a little more restrictive. I was hoping to complete this today, but it might not be until tomorrow.
@zonyitoo commented on GitHub (May 13, 2018):
BTW, what's the best way to create a
ResolverFuturein global? CurrentlyResolverFuture::newreturns aFuture.@bluejekyll commented on GitHub (May 13, 2018):
This is a great question, and your example made me thing this might be a missing function in the library.
Basically, there are a few things that will be needed here:
Now I don't yet know what is best for you at the moment. It might be to clone the ResolverFuture into a current context and then use that cloned version for lookups. I'm hoping that the work I'm doing right now, instigated by this issue, will make Lookup*Futures all sendable, and then the clone of the ResolverFuture won't be necessary (so far this looks promising).
Thanks for your patience with this!
@bluejekyll commented on GitHub (May 13, 2018):
See #460, as it might help... the resolver and proto libraries are compiling. Mind you it's a work in progress, so I may change a bunch of interfaces...
@zonyitoo commented on GitHub (May 13, 2018):
It compiles!! And works great!!!
@zonyitoo commented on GitHub (May 13, 2018):
You can merge that PR now.
Currently I only need the
lookup_ipfunction, so I don't think this API will be changed. :)@bluejekyll commented on GitHub (May 13, 2018):
There are still some issues to figure out in other parts of the library. Once I work through those, I'll merge this in.
@zonyitoo commented on GitHub (May 14, 2018):
Well, it seems that my example to put
ResolverFutureinto a global variable is wrong. I found this error:Which occurs from here.
So, it maybe wrong to use
tokio::runtime::current_thread::Runtime::block_onto create aResolverFuture.Why
ResolverFuture::newhave to return aFutureanyway? I don't see any blocking call in this function.@bluejekyll commented on GitHub (May 14, 2018):
Please take a look at #460 and let me know if that works for you, I added your example and modified it to create a global Resolver thread.r
There are two issues here and the new tokio updates made those apparent. Your example made it clear to me that we need to split the construction of the ResolverFuture into two, one for the runtime, that needs to be a future, and one for a handle to issue requests to that runtime.
I won’t be doing that as part of #460, but will file an issue for the next release.
@zonyitoo commented on GitHub (May 14, 2018):
#460 works!