Fix execute_future not working with tokio::spawn
#166
Conversation
Cargo.toml
Outdated
| @@ -63,7 +63,7 @@ num-bigint-dig = "0.8" | |||
| tracing = "0.1" | |||
| rustls = { version = "0.21", features = ["dangerous_configuration"], optional = true } | |||
| zeroize = { version = "1.6", features = ["zeroize_derive"] } | |||
| tokio = { version = "1.32", features = ["time", "rt"], optional = true } | |||
| tokio = { version = "1.32", features = ["time", "rt", "rt-multi-thread"], optional = true } | |||
There was a problem hiding this comment.
Do we really need to enable the "rt-multi-thread" feature for tokio?
There was a problem hiding this comment.
tokio::block_in_place is guarded by check for rt-multi-thread feature. We can either enable it here or add another feature in sspi to and add it there (with cfg guard in code)
There was a problem hiding this comment.
Oh okay! In that case, I think it’s good as-is, let’s not add more complexity to this. Longer term, I also have a plan for removing the dependency to tokio completely and leave the user spawn a task in their runtime directly.
| dns_resolver = ["dep:trust-dns-resolver", "dep:tokio"] | ||
| dns_resolver = ["dep:trust-dns-resolver", "dep:tokio", "tokio/rt-multi-thread"] |
There was a problem hiding this comment.
This change is good, thank you 👍
| tokio = { version = "1.32", features = ["time", "rt"] } | ||
| tokio = { version = "1.32", features = ["time", "rt", "rt-multi-thread"] } |
There was a problem hiding this comment.
Since the rt-multi-thread feature is now implied by dns_resolver, you can probably remove it from here too?
There was a problem hiding this comment.
#[cfg(any(feature = "dns_resolver", target_os = "macos", target_os = "ios"))]<- you have to enable it for either dns_resolver or macos/ios, so it's required in both places
There was a problem hiding this comment.
It’s probably redundant, but it’s not on you, I’ll merge as-is
In some cases (e.g. when using async IO, like TCP connection) resolving DNS queries for KDC will block when used in
tokio::spawn-ed task. AFAICT this happens because we block thread that is used by IO and timers.This change fixes that with use
block_in_placeso the thread we're on will be marked by tokio as blocked and other task will be moved to the new one.