[commands::run_server_process]: fix race condition #2077
Conversation
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #2077 +/- ##
==========================================
- Coverage 30.90% 30.89% -0.01%
==========================================
Files 52 52
Lines 19912 19914 +2
Branches 9617 9613 -4
==========================================
Hits 6153 6153
+ Misses 7931 7929 -2
- Partials 5828 5832 +4 ☔ View full report in Codecov by Sentry. |
src/commands.rs
Outdated
| let listener = runtime.block_on(async move { | ||
| tokio::net::UnixListener::bind(sp).expect("Failed to bind Unix socket") | ||
| }); |
There was a problem hiding this comment.
| let listener = runtime.block_on(async move { | |
| tokio::net::UnixListener::bind(sp).expect("Failed to bind Unix socket") | |
| }); | |
| let listener = runtime.block_on(async move { | |
| tokio::net::UnixListener::bind(sp) | |
| })?; |
Untested, but perhaps returning the error directly would be better (to preserve the original functionality).
There was a problem hiding this comment.
Works well, I've pushed a fixup with this change.
src/commands.rs
Outdated
| @@ -89,6 +89,11 @@ fn run_server_process(startup_timeout: Option<Duration>) -> Result<ServerStartup | |||
| let runtime = Runtime::new()?; | |||
| let exe_path = env::current_exe()?; | |||
| let workdir = exe_path.parent().expect("executable path has no parent?!"); | |||
|
|
|||
| // Spawn a blocking task to bind the Unix socket | |||
There was a problem hiding this comment.
maybe add a comment to explain that it has to be done here to avoid a race
There was a problem hiding this comment.
Done in latest version.
src/commands.rs
Outdated
| let sp = socket_path.clone(); | ||
| let listener = runtime.block_on(async move { tokio::net::UnixListener::bind(sp) })?; |
There was a problem hiding this comment.
This can be written let listener = tokio::net::UnixListener::bind(&socket_path.clone())?;
There was a problem hiding this comment.
It compiles, but when I run it I get:
$ ./target/debug/sccache --start-server
sccache: Starting the server...
thread 'main' panicked at 'there is no reactor running, must be called from the context of a Tokio 1.x runtime', src/commands.rs:98:20
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
The way I understand it: the function run_server_process is not async. Therefore, you cannot call async functions in it without starting a Tokio runtime(?). tokio::net::UnixListener::bind is an async function. Therefore, you must call it inside a runtime.block_on call so that it runs in a runtime(?).
There was a problem hiding this comment.
The function is not, actually, async. https://docs.rs/tokio/latest/tokio/net/struct.UnixListener.html#method.bind But it does not want to run without a runtime.
So the following would work too, without an async block:
let listener = {
let _guard = runtime.enter();
tokio::net::UnixListener::bind(&socket_path.clone())?
};
There was a problem hiding this comment.
Indeed. I've pushed a fixup with your last suggestion.
|
Latest workflow failed due to clippy complaining about the |
|
many thanks |
Fixes #2076