[GH-ISSUE #456] LookUp Future is not Sendable between threads #196

Closed
opened 2026-03-07 22:44:10 +03:00 by kerem · 13 comments
Owner

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 tokio with ResolverFuture. tokio requires Futures to be Sendable.

Since the ResolverFuture::client_cache and ResolverFuture::with_cache are both private, so I put the ResolverFuture in a global static variable like this:

lazy_static! {
    static ref GLOBAL_DNS_RESOLVER: Mutex<Option<ResolverFuture>> = Mutex::new(None);
}

and then initialize GLOBAL_DNS_RESOLVER in the future of tokio::run.

That works fine except when I wanted to resolve a name:

let resolver = GLOBAL_DNS_RESOLVER.lock()
                                      .unwrap()
                                      .as_ref()
                                      .expect("Haven't initialized DNS resolver yet!");

resolver.lookup_ip(addr)

resolver.lookup_ip returns a LookupIpFuture, which is not sendable between threads:

   |     ^^^^^^^^^^^^ `futures::Future<Item=trust_dns_resolver::lookup::Lookup, Error=trust_dns_resolver::error::ResolveError> + 'static` cannot be sent between threads safely

this is BAD!

I don't want to create a ResolverFuture every 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?

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 `tokio` with `ResolverFuture`. `tokio` requires `Future`s to be `Send`able. Since the `ResolverFuture::client_cache` and `ResolverFuture::with_cache` are both private, so I put the `ResolverFuture` in a global static variable like this: ```rust lazy_static! { static ref GLOBAL_DNS_RESOLVER: Mutex<Option<ResolverFuture>> = Mutex::new(None); } ``` and then initialize `GLOBAL_DNS_RESOLVER` in the future of `tokio::run`. That works fine except when I wanted to resolve a name: ```rust let resolver = GLOBAL_DNS_RESOLVER.lock() .unwrap() .as_ref() .expect("Haven't initialized DNS resolver yet!"); resolver.lookup_ip(addr) ``` `resolver.lookup_ip` returns a `LookupIpFuture`, which is not sendable between threads: ``` | ^^^^^^^^^^^^ `futures::Future<Item=trust_dns_resolver::lookup::Lookup, Error=trust_dns_resolver::error::ResolveError> + 'static` cannot be sent between threads safely ``` this is BAD! I don't want to create a `ResolverFuture` every 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?
kerem 2026-03-07 22:44:10 +03:00
Author
Owner

@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.

<!-- gh-comment-id:388442080 --> @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.
Author
Owner

@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.

<!-- gh-comment-id:388584201 --> @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.
Author
Owner

@zonyitoo commented on GitHub (May 13, 2018):

Glad to hear that!!

<!-- gh-comment-id:388608069 --> @zonyitoo commented on GitHub (May 13, 2018): Glad to hear that!!
Author
Owner

@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.

<!-- gh-comment-id:388608201 --> @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.
Author
Owner

@zonyitoo commented on GitHub (May 13, 2018):

BTW, what's the best way to create a ResolverFuture in global? Currently ResolverFuture::new returns a Future.

<!-- gh-comment-id:388608416 --> @zonyitoo commented on GitHub (May 13, 2018): BTW, what's the best way to create a `ResolverFuture` in global? Currently `ResolverFuture::new` returns a `Future`.
Author
Owner

@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:

  • A global tokio::Runtime, probably running in it's own thread
  • Then you can create a new ResolverFuture in a global function.
    • using the Runtime::block_on() you can get a reference to the ResolverFuture
    • technically the ResolverFuture reference at that point is a Handle to the actual resolver future that is spawned to the Runtime.
    • then store the ResolverFuture in the static like you have

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!

<!-- gh-comment-id:388609178 --> @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: - A global tokio::Runtime, probably running in it's own thread - Then you can create a new ResolverFuture in a global function. - using the Runtime::block_on() you can get a reference to the ResolverFuture - technically the ResolverFuture reference at that point is a Handle to the actual resolver future that is spawned to the Runtime. - then store the ResolverFuture in the static like you have 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!
Author
Owner

@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...

<!-- gh-comment-id:388609863 --> @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...
Author
Owner

@zonyitoo commented on GitHub (May 13, 2018):

It compiles!! And works great!!!

<!-- gh-comment-id:388633336 --> @zonyitoo commented on GitHub (May 13, 2018): It compiles!! And works great!!!
Author
Owner

@zonyitoo commented on GitHub (May 13, 2018):

You can merge that PR now.
Currently I only need the lookup_ip function, so I don't think this API will be changed. :)

<!-- gh-comment-id:388633554 --> @zonyitoo commented on GitHub (May 13, 2018): You can merge that PR now. Currently I only need the `lookup_ip` function, so I don't think this API will be changed. :)
Author
Owner

@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.

<!-- gh-comment-id:388647627 --> @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.
Author
Owner

@zonyitoo commented on GitHub (May 14, 2018):

Well, it seems that my example to put ResolverFuture into a global variable is wrong. I found this error:

thread 'tokio-runtime-worker-0' panicked at 'Multiple executors at once: EnterError { _a: () }', libcore/result.rs:945:5

Which occurs from here.

So, it maybe wrong to use tokio::runtime::current_thread::Runtime::block_on to create a ResolverFuture.

Why ResolverFuture::new have to return a Future anyway? I don't see any blocking call in this function.

<!-- gh-comment-id:388850707 --> @zonyitoo commented on GitHub (May 14, 2018): Well, it seems that my example to put `ResolverFuture` into a global variable is wrong. I found this error: ``` thread 'tokio-runtime-worker-0' panicked at 'Multiple executors at once: EnterError { _a: () }', libcore/result.rs:945:5 ``` Which occurs from [here](https://github.com/tokio-rs/tokio/blob/246548384558ee0b01252935448cc23b7fd767e2/src/runtime/current_thread/runtime.rs#L130). So, it maybe wrong to use `tokio::runtime::current_thread::Runtime::block_on` to create a `ResolverFuture`. Why `ResolverFuture::new` have to return a `Future` anyway? I don't see any blocking call in this function.
Author
Owner

@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

Why ResolverFuture::new have to return a Future anyway? I don't see any blocking call in this function.

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.

<!-- gh-comment-id:388854461 --> @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 > Why ResolverFuture::new have to return a Future anyway? I don't see any blocking call in this function. 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.
Author
Owner

@zonyitoo commented on GitHub (May 14, 2018):

#460 works!

<!-- gh-comment-id:388871504 --> @zonyitoo commented on GitHub (May 14, 2018): #460 works!
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
starred/hickory-dns#196
No description provided.