Repository: actix/actix (language: rust)

Issue:
problem dependency with trust-dns-proto
I'm using mongodb 9.0.1 that use:

```toml
trust-dns-proto = "0.18.0"
trust-dns-resolver = "0.18.0"
```
https://github.com/mongodb/mongo-rust-driver/blob/bfd69496baf56ba89668f589dfed8733aa24c42d/Cargo.toml#L35

And actix 0.9.0 that use:
```rust
trust-dns-proto = { version = "=0.18.0-alpha.2", optional = true, default-features = false }
trust-dns-resolver = { version = "=0.18.0-alpha.2", optional = true, default-features = false }
```
https://github.com/actix/actix/blob/5eed1043438b9b5d62fec103db3665657f9c61bc/Cargo.toml#L58

It's produce the following error message:

```none
error: failed to select a version for `trust-dns-proto`.
    ... required by package `mongodb v0.9.1`
    ... which is depended on by `blender v1.0.5 (/home/stargateur/blender)`
versions that meet the requirements `^0.18.0` are: 0.18.1, 0.18.0

all possible versions conflict with previously selected packages.

  previously selected package `trust-dns-proto v0.18.0-alpha.2`
    ... which is depended on by `actix v0.9.0`
    ... which is depended on by `blender v1.0.5 (/home/stargateur/blender)`

failed to select a version for `trust-dns-proto` which could resolve this conflict
```


Discussion / hints:
We're aware of that and in roadmap of 0.10 :)
@JohnTitor I'm not good with semver tag but shouldn't actix 9.0 be 0.9.0-alpha.3 ?
> I'm not good with semver tag but shouldn't actix 9.0 be 0.9.0-alpha.3 ?

Uhm? Where do you refer to?
actix versioning https://crates.io/crates/actix/versions.

I don't understand why actix release a 0.9.0 not compatible with other crates that use non alpha release of trust-dns-proto. I mean I just can't compile with 0.9.0 at this point, so 0.9.0 is unusable.
> I don't understand why actix release a 9.0 not compatible with other crates that use non alpha release of trust-dns-proto.

I believe you mean 0.9.0, not 9.0 :) Yeah, it's not ideal that 0.9.0 relies on their pinned versions.

> I mean I just can't compile with 9.0 at this point, so 9.0 is unusable.

0.9.0 is still usable, however, when you can avoid conflicts. I'd like to make a new alpha-release, i.e. 0.10.0 (alpha) with updating their deps versions since it'll also break backward compatibility. Unfortunately, I really don't have bandwidth enough now so the work will be late :/ Any contributions are welcome!
Well, I struggle to just trying to use/understand actix so I will not be very effective :p but I will clearly keep that in mind.

mongodb issues I found:

 - https://jira.mongodb.org/browse/RUST-287
 - https://jira.mongodb.org/browse/RUST-290

Their will update to trust-dns-proto 0.19, but the problem come from actix so... that will happen again. (well, even if I think cargo is also the problem... semver is strange).
The conflict occurs between 0.18.0 and 0.18.0-alpha.2 so indeed using mongodb 0.9.0 is an avoidance.

You can try:
```toml
[dependencies]
actix = "0.9.0"
mongodb = "=0.9.0"
```
> No worries, but be on the lookout for this fix, because 0.9.0 uses old trust-dns-* versions that still rely on tokio 0.1 (so dependency bloat) [link](https://jira.mongodb.org/browse/RUST-287?focusedCommentId=2881059&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-2881059)

Well, that will do it for me for the moment, thanks for the tips it's worked, I let you choice when to close this issue.
I have started a migration that does compile, but couldn't test it properly so far. If anyone can help testing and let me know of any problems would be of great help.

My migration attempt is available on [a branch on my fork](https://github.com/Jonathas-Conceicao/actix/tree/topic/upgrade_trust_dns).

One important diference from the older implementation to the one I currently have is: the old implementation would use the Actor's context as a "background runner" to resolve spawned by `trust_dns_resolver::AsyncResolver`, the new API makes it a little hard to reproduce the same behavior, so I decided to let it spawn it's futures in to the current runtime directly since it's much simple, however I'm not sure if this is a problem or not. If this isn't desirable for some reason, I think the `Addr<Resolver>` can be made into Runtime for the `trust_dns_resolver::AsyncResolver`, but it would add some considerable complexity and higher load of message passing.

Fix patch:
diff --git a/CHANGES.md b/CHANGES.md
index 72f4999bb..b61bbb799 100644
--- a/CHANGES.md
+++ b/CHANGES.md
@@ -12,6 +12,12 @@
 
 * Allow return of any `T: 'static` on `ResponseActFuture`. (#310)
 
+### Changed
+
+* Upgrade `trust-dns-proto` to 0.19
+
+* Upgrade `trust-dns-resolver` to 0.19
+
 ## [0.9.0] 2019-12-20
 
 ### Fixed
diff --git a/Cargo.toml b/Cargo.toml
index ca521e337..f87f81256 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -55,8 +55,8 @@ tokio-util   = { version = "0.2", features = ["full"] }
 actix-http = { version = "1.0.1", optional = true }
 
 # dns resolver
-trust-dns-proto = { version = "=0.18.0-alpha.2", optional = true, default-features = false }
-trust-dns-resolver = { version = "=0.18.0-alpha.2", optional = true, default-features = false }
+trust-dns-proto = { version = "0.19", optional = true, default-features = false, features = ["tokio-runtime"] }
+trust-dns-resolver = { version = "0.19", optional = true, default-features = false, features = ["tokio-runtime", "system-config"] }
 
 [dev-dependencies]
 doc-comment = "0.3"
diff --git a/src/actors/resolver.rs b/src/actors/resolver.rs
index a6c6cadfa..a23d5888a 100644
--- a/src/actors/resolver.rs
+++ b/src/actors/resolver.rs
@@ -3,31 +3,27 @@
 //! ## Example
 //!
 //! ```rust
-//! #![recursion_limit="128"]
-//! use futures::{future, FutureExt};
-//! use actix::prelude::*;
 //! use actix::actors::resolver;
+//! use actix::prelude::*;
 //!
 //! #[actix_rt::main]
 //! async fn main() {
-//!         actix_rt::spawn(async {
-//!             let resolver = resolver::Resolver::from_registry();
-//!
-//!             let addrs = resolver.send(
-//!                  resolver::Connect::host("localhost")).await;
-//!
-//!             println!("RESULT: {:?}", addrs);
-//!             System::current().stop();
-//!        });
+//!     Arbiter::spawn(async {
+//!         let resolver = resolver::Resolver::from_registry();
+//!         let addrs = resolver
+//!             .send(resolver::Resolve::host("localhost"))
+//!             .await
+//!             .unwrap();
 //!
-//!         actix_rt::spawn(async {
-//!             let resolver = resolver::Resolver::from_registry();
+//!         println!("RESULT: {:?}", addrs);
+//!     });
 //!
-//!             let stream = resolver.send(
-//!                  resolver::Connect::host_and_port("localhost", 5000)).await;
-//!
-//!             println!("RESULT: {:?}", stream);
-//!        });
+//!     let resolver = resolver::Resolver::from_registry();
+//!     let addrs = resolver
+//!         .send(resolver::Connect::host_and_port("localhost", 5000))
+//!         .await
+//!         .unwrap();
+//!     println!("RESULT: {:?}", addrs);
 //! }
 //! ```
 use std::collections::VecDeque;
@@ -42,11 +38,10 @@ use derive_more::Display;
 use log::warn;
 use tokio::net::TcpStream;
 use trust_dns_resolver::config::{ResolverConfig, ResolverOpts};
-use trust_dns_resolver::AsyncResolver;
-use trust_dns_resolver::BackgroundLookupIp;
+use trust_dns_resolver::TokioAsyncResolver as AsyncResolver;
+use trust_dns_resolver::{error::ResolveError, lookup_ip::LookupIp};
 
 use crate::clock::Delay;
-use crate::fut::wrap_future;
 use crate::fut::ActorFuture;
 use crate::fut::Either;
 use crate::prelude::*;
@@ -164,49 +159,52 @@ impl Resolver {
             err: None,
         }
     }
-
-    fn start_resolver<F>(
-        &self,
-        ctx: &mut <Self as Actor>::Context,
-        parts: (AsyncResolver, F),
-    ) -> AsyncResolver
-    where
-        F: 'static + Future<Output = ()>,
-    {
-        ctx.spawn(wrap_future::<_, Self>(parts.1));
-        parts.0
-    }
 }
 
 impl Actor for Resolver {
     type Context = Context<Self>;
-
     fn started(&mut self, ctx: &mut Self::Context) {
-        // AsyncResolver::new() returns the AsyncResolver itself, plus an anonymous
-        // future which gets spawned as a background task for doing DNS
-        // resolution. So we use our little `start_resolver` wrapper to spawn
-        // the background task (which gets cleaned up automatically if no
-        // outstanding AsyncResolvers still have a handle to it).
-        let resolver = if let Some(cfg) = self.cfg.take() {
-            self.start_resolver(ctx, AsyncResolver::new(cfg.0, cfg.1))
-        } else {
-            match AsyncResolver::from_system_conf() {
-                Ok(resolver) => self.start_resolver(ctx, resolver),
-                Err(err) => {
-                    warn!("Can not create system dns resolver: {}", err);
-                    self.start_resolver(
-                        ctx,
-                        AsyncResolver::new(
-                            ResolverConfig::default(),
-                            ResolverOpts::default(),
-                        ),
-                    )
-                }
-            }
-        };
-
-        // Keep the resolver itself.
-        self.resolver = Some(resolver);
+        let cfg = self.cfg.take();
+        ctx.wait(
+            async move { cfg }
+                .into_actor(self)
+                .then(
+                    |cfg, this, _| -> Pin<Box<dyn ActorFuture<Actor = Self, Output = _>>> {
+                        if let Some(cfg) = cfg {
+                            return Box::pin(
+                                AsyncResolver::tokio(cfg.0, cfg.1).into_actor(this),
+                            );
+                        }
+                        Box::pin(
+                            async {
+                                match AsyncResolver::from_system_conf(
+                                    tokio::runtime::Handle::current(),
+                                )
+                                .await
+                                {
+                                    Ok(resolver) => Ok(resolver),
+                                    Err(err) => {
+                                        warn!(
+                                            "Can not create system dns resolver: {}",
+                                            err
+                                        );
+                                        AsyncResolver::tokio(
+                                            ResolverConfig::default(),
+                                            ResolverOpts::default(),
+                                        )
+                                        .await
+                                    }
+                                }
+                            }
+                            .into_actor(this),
+                        )
+                    },
+                )
+                .map(|resolver_res, this, _| {
+                    // Keep the resolver itself.
+                    this.resolver = Some(resolver_res.unwrap());
+                }),
+        );
     }
 }
 
@@ -269,9 +267,11 @@ impl Handler<ConnectAddr> for Resolver {
     }
 }
 
+type LookupIpFuture = Pin<Box<dyn Future<Output = Result<LookupIp, ResolveError>>>>;
+
 /// A resolver future.
 struct ResolveFut {
-    lookup: Option<BackgroundLookupIp>,
+    lookup: Option<LookupIpFuture>,
     port: u16,
     addrs: Option<VecDeque<SocketAddr>>,
     error: Option<ResolverError>,
@@ -301,7 +301,15 @@ impl ResolveFut {
             match ResolveFut::parse(addr.as_ref(), port) {
                 Ok((host, port)) => ResolveFut {
                     port,
-                    lookup: Some(resolver.lookup_ip(host)),
+                    lookup: {
+                        // Clone data so it can be moved to async block
+                        let resolver_clone = resolver.clone();
+                        let host = host.to_string();
+                        Some(Box::pin(async move {
+                            let resolver = resolver_clone;
+                            resolver.lookup_ip(host).await
+                        }))
+                    },
                     addrs: None,
                     error: None,
                     error2: None,


Test patch:
diff --git a/tests/test_actors.rs b/tests/test_actors.rs
index e858fece6..e7870bbd9 100644
--- a/tests/test_actors.rs
+++ b/tests/test_actors.rs
@@ -3,16 +3,17 @@ use actix::prelude::*;
 
 #[actix_rt::test]
 async fn test_resolver() {
-    Arbiter::spawn(async {
-        let resolver = resolver::Resolver::from_registry();
-        let _ = resolver.send(resolver::Resolve::host("localhost")).await;
-        System::current().stop();
-    });
-
     Arbiter::spawn(async {
         let resolver = resolver::Resolver::from_registry();
         let _ = resolver
-            .send(resolver::Connect::host("localhost:5000"))
-            .await;
+            .send(resolver::Resolve::host("localhost"))
+            .await
+            .unwrap();
     });
+
+    let resolver = resolver::Resolver::from_registry();
+    let _ = resolver
+        .send(resolver::Connect::host("localhost:5000"))
+        .await
+        .unwrap();
 }