Skip to content

Commit 8968eba

Browse files
alexcrichtonluser
authored andcommitted
Remove dependency on named_pipe
I was sporadically receiving a segfault locally when trying to debug issues on Windows and in tracking this down I discovered blackbeam/named_pipe#3 which leads to segfaults locally on startup. This switches the one use case to the relevant functionality in `mio-named-pipes` (already pulled in as part of `tokio-process`) and then otherwise mirrors the same logic as the Unix version, just waiting for a byte with a timeout.
1 parent 81edc56 commit 8968eba

File tree

8 files changed

+53
-94
lines changed

8 files changed

+53
-94
lines changed

.travis.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ language: rust
22
cache: cargo
33

44
rust:
5-
- 1.13.0
5+
- 1.16.0
66
- stable
77
- beta
88
- nightly

Cargo.lock

Lines changed: 1 addition & 11 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Cargo.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,8 +47,8 @@ tokio-uds = "0.1"
4747

4848
[target.'cfg(windows)'.dependencies]
4949
kernel32-sys = "0.2.2"
50-
named_pipe = "0.2.2"
5150
winapi = "0.2"
51+
mio-named-pipes = "0.1"
5252

5353
[features]
5454
default = []

README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ Sccache can also be used with local storage instead of remote.
1313
Requirements
1414
------------
1515

16-
sccache is a [Rust](https://www.rust-lang.org/) program. Building it requires `cargo` (and thus `rustc`). sccache currently requires **Rust 1.13**.
16+
sccache is a [Rust](https://www.rust-lang.org/) program. Building it requires `cargo` (and thus `rustc`). sccache currently requires **Rust 1.16**.
1717

1818
We recommend you install Rust via [Rustup](https://rustup.rs/). The generated binaries can be built so that they are very portable, see [scripts/build-release.sh](scripts/build-release.sh).
1919

src/commands.rs

Lines changed: 43 additions & 73 deletions
Original file line numberDiff line numberDiff line change
@@ -192,87 +192,57 @@ fn redirect_error_log() -> Result<()> {
192192
}
193193

194194
/// Re-execute the current executable as a background server.
195-
///
196-
/// `std::process::Command` doesn't expose a way to create a
197-
/// detatched process on Windows, so we have to roll our own.
198-
/// TODO: remove this all when `CommandExt::creation_flags` hits stable:
199-
/// https://github.com/rust-lang/rust/issues/37827
200195
#[cfg(windows)]
201196
fn run_server_process() -> Result<ServerStartup> {
202-
use kernel32;
203-
use named_pipe::PipeOptions;
204-
use std::io::{Read, Error};
205-
use std::os::windows::ffi::OsStrExt;
206-
use std::mem;
207-
use std::ptr;
197+
use futures::Future;
198+
use mio_named_pipes::NamedPipe;
199+
use std::os::windows::process::CommandExt;
200+
use std::time::Duration;
201+
use tokio_core::io::read_exact;
202+
use tokio_core::reactor::{Core, Timeout, PollEvented};
208203
use uuid::Uuid;
209-
use winapi::minwindef::{TRUE,FALSE,LPVOID,DWORD};
210-
use winapi::processthreadsapi::{PROCESS_INFORMATION,STARTUPINFOW};
211-
use winapi::winbase::{CREATE_UNICODE_ENVIRONMENT,DETACHED_PROCESS,CREATE_NEW_PROCESS_GROUP};
204+
use winapi::{DETACHED_PROCESS, CREATE_NEW_PROCESS_GROUP};
212205

213206
trace!("run_server_process");
214-
// Create a pipe to get startup status back from the server.
207+
208+
// Create a mini event loop and register our named pipe server
209+
let mut core = Core::new()?;
210+
let handle = core.handle();
215211
let pipe_name = format!(r"\\.\pipe\{}", Uuid::new_v4().simple());
216-
let server = PipeOptions::new(&pipe_name).single()?;
212+
let server = NamedPipe::new(&pipe_name)?;
213+
let server = PollEvented::new(server, &handle)?;
214+
215+
// Connect a client to our server, and we'll wait below if it's still in
216+
// progress.
217+
match server.get_ref().connect() {
218+
Ok(()) => {}
219+
Err(ref e) if e.kind() == io::ErrorKind::WouldBlock => {}
220+
Err(e) => return Err(e.into()),
221+
}
222+
223+
// Spawn a server which should come back and connect to us
217224
let exe_path = env::current_exe()?;
218-
let mut exe = OsStr::new(&exe_path)
219-
.encode_wide()
220-
.chain(Some(0u16))
221-
.collect::<Vec<u16>>();
222-
// Collect existing env vars + extra into an environment block.
223-
let mut envp = {
224-
let mut v = vec!();
225-
let extra_vars = vec![
226-
(OsString::from("SCCACHE_START_SERVER"), OsString::from("1")),
227-
(OsString::from("SCCACHE_STARTUP_NOTIFY"), OsString::from(&pipe_name)),
228-
(OsString::from("RUST_BACKTRACE"), OsString::from("1")),
229-
];
230-
for (key, val) in env::vars_os().chain(extra_vars) {
231-
v.extend(key.encode_wide().chain(Some('=' as u16)).chain(val.encode_wide()).chain(Some(0)));
232-
}
233-
v.push(0);
234-
v
235-
};
236-
let mut pi = PROCESS_INFORMATION {
237-
hProcess: ptr::null_mut(),
238-
hThread: ptr::null_mut(),
239-
dwProcessId: 0,
240-
dwThreadId: 0,
241-
};
242-
let mut si: STARTUPINFOW = unsafe { mem::zeroed() };
243-
si.cb = mem::size_of::<STARTUPINFOW>() as DWORD;
244-
if unsafe { kernel32::CreateProcessW(exe.as_mut_ptr(),
245-
ptr::null_mut(),
246-
ptr::null_mut(),
247-
ptr::null_mut(),
248-
FALSE,
249-
CREATE_UNICODE_ENVIRONMENT | DETACHED_PROCESS | CREATE_NEW_PROCESS_GROUP,
250-
envp.as_mut_ptr() as LPVOID,
251-
ptr::null(),
252-
&mut si,
253-
&mut pi) == TRUE } {
254-
unsafe {
255-
kernel32::CloseHandle(pi.hProcess);
256-
kernel32::CloseHandle(pi.hThread);
225+
let _child = process::Command::new(exe_path)
226+
.env("SCCACHE_START_SERVER", "1")
227+
.env("SCCACHE_STARTUP_NOTIFY", &pipe_name)
228+
.env("RUST_BACKTRACE", "1")
229+
.creation_flags(DETACHED_PROCESS | CREATE_NEW_PROCESS_GROUP)
230+
.spawn()?;
231+
232+
let result = read_exact(server, [0u8]).map(|(_socket, byte)| {
233+
if byte[0] == 0 {
234+
ServerStartup::Ok
235+
} else {
236+
let err = format!("Server startup failed: {}", byte[0]).into();
237+
ServerStartup::Err(err)
257238
}
258-
} else {
259-
return Err(Error::last_os_error().into())
260-
}
261-
// Wait for a connection on the pipe.
262-
let mut pipe = match server.wait_ms(SERVER_STARTUP_TIMEOUT_MS)? {
263-
Ok(pipe) => pipe,
264-
Err(_) => return Ok(ServerStartup::TimedOut),
265-
};
266-
// It would be nice to have a read timeout here.
267-
let mut buffer = [0; 1];
268-
pipe.read_exact(&mut buffer)?;
269-
if buffer[0] == 0 {
270-
info!("Server started up successfully");
271-
Ok(ServerStartup::Ok)
272-
} else {
273-
//TODO: send error messages over the socket as well.
274-
error!("Server startup failed: {}", buffer[0]);
275-
Ok(ServerStartup::Err(format!("Server startup failed: {}", buffer[0]).into()))
239+
});
240+
241+
let timeout = Duration::from_millis(SERVER_STARTUP_TIMEOUT_MS.into());
242+
let timeout = Timeout::new(timeout, &handle)?.map(|()| ServerStartup::TimedOut);
243+
match core.run(result.select(timeout)) {
244+
Ok((e, _other)) => Ok(e),
245+
Err((e, _other)) => Err(e).chain_err(|| "failed waiting for server to start"),
276246
}
277247
}
278248

src/main.rs

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,6 @@
1212
// See the License for the specific language governing permissions and
1313
// limitations under the License.
1414

15-
#![cfg_attr(feature = "unstable", feature(windows_process_extensions))]
16-
1715
extern crate app_dirs;
1816
extern crate chrono;
1917
extern crate clap;
@@ -38,7 +36,7 @@ extern crate lru_disk_cache;
3836
extern crate fern;
3937
extern crate libc;
4038
#[cfg(windows)]
41-
extern crate named_pipe;
39+
extern crate mio_named_pipes;
4240
extern crate number_prefix;
4341
extern crate protobuf;
4442
extern crate regex;

src/mock_command.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -192,15 +192,15 @@ impl RunCommand for AsyncCommand {
192192
self
193193
}
194194

195-
#[cfg(all(windows, feature = "unstable"))]
195+
#[cfg(windows)]
196196
fn no_console(&mut self) -> &mut AsyncCommand {
197197
use std::os::windows::process::CommandExt;
198198
const CREATE_NO_WINDOW: u32 = 0x08000000;
199199
self.inner.creation_flags(CREATE_NO_WINDOW);
200200
self
201201
}
202202

203-
#[cfg(not(all(windows, feature = "unstable")))]
203+
#[cfg(unix)]
204204
fn no_console(&mut self) -> &mut AsyncCommand {
205205
self
206206
}

src/server.rs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -98,12 +98,13 @@ fn notify_server_startup(name: &Option<OsString>, success: bool) -> io::Result<(
9898

9999
#[cfg(windows)]
100100
fn notify_server_startup(name: &Option<OsString>, success: bool) -> io::Result<()> {
101-
use named_pipe::PipeClient;
101+
use std::fs::OpenOptions;
102+
102103
let name = match *name {
103104
Some(ref s) => s,
104105
None => return Ok(()),
105106
};
106-
let pipe = try!(PipeClient::connect(name));
107+
let pipe = try!(OpenOptions::new().write(true).read(true).open(name));
107108
notify_server_startup_internal(pipe, success)
108109
}
109110

0 commit comments

Comments
 (0)