Skip to content

Commit a36d9a8

Browse files
MikailBagMikailBag
committed
fix tracing
1 parent bca89a2 commit a36d9a8

File tree

9 files changed

+191
-162
lines changed

9 files changed

+191
-162
lines changed

‎Cargo.toml

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,6 @@ authors = ["Mikail Bagishov <bagishov.mikail@yandex.ru>"]
55
edition = "2018"
66

77
[dependencies]
8-
libc = "0.2.86"
9-
errno = "0.2.7"
108
rand = "0.8.3"
119
serde = { version = "1.0.123", features = ["derive"] }
1210
serde_json = "1.0.62"
@@ -17,7 +15,6 @@ once_cell = "1.5.2"
1715
futures-util = "0.3.12"
1816
tokio = { version = "1.2.0", features = ["net"] }
1917
tracing = "0.1.23"
20-
itoa = "0.4.7"
2118

2219
[target.'cfg(target_os="linux")'.dependencies]
2320
tiny-nix-ipc = { git = "https://github.com/myfreeweb/tiny-nix-ipc", features = ["ser_json", "zero_copy"] }
@@ -26,6 +23,7 @@ libc = "0.2.86"
2623
errno = "0.2.7"
2724
serde_json = "1.0.62"
2825
tokio = { version = "1.2.0", features = ["net"] }
26+
itoa = "0.4.7"
2927

3028
[target.'cfg(target_os="windows")'.dependencies]
3129
winapi = { version = "0.3.9", features = ["std", "processthreadsapi", "jobapi2", "errhandlingapi",

‎minion-tests/src/main.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,8 @@ static WORKER_ENV_NAME: &str = "__MINION_ROLE_IS_WORKER__";
6464
static TEST_ENV_NAME: &str = "__MINION_ROLE_IS_TEST__";
6565
fn main() {
6666
tracing_subscriber::fmt()
67-
.with_writer(std::io::stdout)
67+
.pretty()
68+
.with_writer(std::io::stderr)
6869
.init();
6970
let test_cases = &*tests::TESTS;
7071
let role = get_role();

‎minion-tests/src/worker.rs

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -58,10 +58,6 @@ async fn inner_main(test_cases: &[&'static dyn TestCase]) {
5858
}
5959

6060
pub fn main(test_cases: &[&'static dyn TestCase]) {
61-
tracing_subscriber::fmt()
62-
.pretty()
63-
.with_writer(std::io::stderr)
64-
.init();
6561
let rt = tokio::runtime::Builder::new_current_thread()
6662
.enable_all()
6763
.build()

‎src/windows/child.rs

Lines changed: 5 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,16 @@
11
use crate::windows::{
22
spawn::{spawn, ChildParams, Stdio},
3+
util::OwnedHandle,
34
Error, ReadPipe, WindowsSandbox, WritePipe,
45
};
5-
use winapi::um::{handleapi::CloseHandle, winnt::HANDLE};
66

77
#[derive(Debug)]
88
pub struct WindowsChildProcess {
99
/// Handle to winapi Process object
10-
child: HANDLE,
10+
child: OwnedHandle,
1111

1212
/// Handle to main process of child
13-
main_thread: HANDLE,
13+
main_thread: OwnedHandle,
1414

1515
/// Stdin
1616
stdin: Option<WritePipe>,
@@ -20,15 +20,6 @@ pub struct WindowsChildProcess {
2020
stderr: Option<ReadPipe>,
2121
}
2222

23-
impl Drop for WindowsChildProcess {
24-
fn drop(&mut self) {
25-
unsafe {
26-
CloseHandle(self.main_thread);
27-
CloseHandle(self.child);
28-
}
29-
}
30-
}
31-
3223
impl WindowsChildProcess {
3324
pub(in crate::windows) fn create_process(
3425
options: crate::ChildProcessOptions<WindowsSandbox>,
@@ -46,8 +37,8 @@ impl WindowsChildProcess {
4637
)?;
4738

4839
Ok(WindowsChildProcess {
49-
child: info.hProcess,
50-
main_thread: info.hThread,
40+
child: OwnedHandle::new(info.hProcess),
41+
main_thread: OwnedHandle::new(info.hThread),
5142
stdin: child_stdin,
5243
stdout: child_stdout,
5344
stderr: child_stderr,

‎src/windows/pipe.rs

Lines changed: 21 additions & 75 deletions
Original file line numberDiff line numberDiff line change
@@ -1,102 +1,49 @@
1-
use crate::windows::{Cvt, Error};
1+
use crate::windows::{util::OwnedHandle, Cvt, Error};
22
use std::os::windows::io::{FromRawHandle, IntoRawHandle, RawHandle};
33
use winapi::{
44
shared::minwindef::TRUE,
5-
um::{
6-
handleapi::CloseHandle, minwinbase::SECURITY_ATTRIBUTES, namedpipeapi::CreatePipe,
7-
winnt::HANDLE,
8-
},
5+
um::{minwinbase::SECURITY_ATTRIBUTES, namedpipeapi::CreatePipe},
96
};
107

118
#[derive(Debug)]
129
pub struct ReadPipe {
13-
handle: HANDLE,
10+
handle: OwnedHandle,
1411
}
1512

16-
unsafe impl Send for ReadPipe {}
17-
unsafe impl Sync for ReadPipe {}
18-
1913
impl IntoRawHandle for ReadPipe {
2014
fn into_raw_handle(self) -> RawHandle {
21-
let h = self.handle;
22-
std::mem::forget(self);
23-
h
15+
self.handle.into_inner()
2416
}
2517
}
2618

2719
impl FromRawHandle for ReadPipe {
2820
unsafe fn from_raw_handle(handle: RawHandle) -> Self {
29-
ReadPipe { handle }
30-
}
31-
}
32-
33-
impl std::io::Read for ReadPipe {
34-
fn read(&mut self, mut buf: &mut [u8]) -> std::io::Result<usize> {
35-
if buf.len() > i32::max_value() as usize {
36-
buf = &mut buf[..(i32::max_value() as usize)]
37-
}
38-
let mut read_cnt = 0;
39-
let res = unsafe {
40-
winapi::um::fileapi::ReadFile(
41-
self.handle,
42-
buf.as_mut_ptr().cast(),
43-
buf.len() as u32,
44-
&mut read_cnt,
45-
std::ptr::null_mut(),
46-
)
47-
};
48-
49-
if res == 0 {
50-
return Err(std::io::Error::last_os_error());
21+
ReadPipe {
22+
handle: OwnedHandle::new(handle),
5123
}
52-
Ok(read_cnt as usize)
5324
}
5425
}
5526

56-
impl Drop for ReadPipe {
57-
fn drop(&mut self) {
58-
unsafe {
59-
CloseHandle(self.handle);
60-
}
27+
impl std::io::Read for ReadPipe {
28+
fn read(&mut self, buf: &mut [u8]) -> std::io::Result<usize> {
29+
self.handle.read(buf)
6130
}
6231
}
6332

6433
#[derive(Debug)]
6534
pub struct WritePipe {
66-
handle: HANDLE,
35+
handle: OwnedHandle,
6736
}
6837

69-
unsafe impl Send for WritePipe {}
70-
unsafe impl Sync for WritePipe {}
71-
7238
impl IntoRawHandle for WritePipe {
7339
fn into_raw_handle(self) -> RawHandle {
74-
let h = self.handle;
75-
std::mem::forget(self);
76-
h
40+
self.handle.into_inner()
7741
}
7842
}
7943

8044
impl std::io::Write for WritePipe {
81-
fn write(&mut self, mut buf: &[u8]) -> std::io::Result<usize> {
82-
if buf.len() > (i32::max_value() as usize) {
83-
buf = &buf[..(i32::max_value() as usize)];
84-
}
85-
let mut written_cnt = 0;
86-
let res = unsafe {
87-
winapi::um::fileapi::WriteFile(
88-
self.handle,
89-
buf.as_ptr().cast(),
90-
buf.len() as u32,
91-
&mut written_cnt,
92-
std::ptr::null_mut(),
93-
)
94-
};
95-
if res != 0 {
96-
Ok(written_cnt as usize)
97-
} else {
98-
Err(std::io::Error::last_os_error())
99-
}
45+
fn write(&mut self, buf: &[u8]) -> std::io::Result<usize> {
46+
self.handle.write(buf)
10047
}
10148

10249
fn flush(&mut self) -> std::io::Result<()> {
@@ -105,14 +52,6 @@ impl std::io::Write for WritePipe {
10552
}
10653
}
10754

108-
impl Drop for WritePipe {
109-
fn drop(&mut self) {
110-
unsafe {
111-
CloseHandle(self.handle);
112-
}
113-
}
114-
}
115-
11655
pub(in crate::windows) enum InheritKind {
11756
Allow,
11857
}
@@ -133,5 +72,12 @@ pub(in crate::windows) fn make(inherit: InheritKind) -> Result<(ReadPipe, WriteP
13372
0,
13473
))?;
13574
}
136-
Ok((ReadPipe { handle: read }, WritePipe { handle: write }))
75+
Ok((
76+
ReadPipe {
77+
handle: OwnedHandle::new(read),
78+
},
79+
WritePipe {
80+
handle: OwnedHandle::new(write),
81+
},
82+
))
13783
}

‎src/windows/sandbox.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,8 +23,8 @@ impl WindowsSandbox {
2323
impl crate::Sandbox for WindowsSandbox {
2424
type Error = Error;
2525

26-
fn id(&self) -> &str {
27-
todo!()
26+
fn id(&self) -> String {
27+
self.id.clone()
2828
}
2929

3030
fn kill(&self) -> Result<(), Self::Error> {

‎src/windows/spawn.rs

Lines changed: 49 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
use crate::{
22
windows::{
33
pipe::{self, ReadPipe, WritePipe},
4+
util::OwnedHandle,
45
Cvt, Error, WindowsSandbox,
56
},
67
InputSpecificationData, OutputSpecificationData,
@@ -17,7 +18,7 @@ use winapi::{
1718
shared::{minwindef::TRUE, winerror::ERROR_INSUFFICIENT_BUFFER},
1819
um::{
1920
errhandlingapi::GetLastError,
20-
handleapi::{CloseHandle, INVALID_HANDLE_VALUE},
21+
handleapi::INVALID_HANDLE_VALUE,
2122
minwinbase::SECURITY_ATTRIBUTES,
2223
processthreadsapi::{
2324
CreateProcessW, DeleteProcThreadAttributeList, GetCurrentProcess,
@@ -34,46 +35,49 @@ use winapi::{
3435
};
3536

3637
pub(in crate::windows) struct Stdio {
37-
pub stdin: HANDLE,
38-
pub stdout: HANDLE,
39-
pub stderr: HANDLE,
38+
pub stdin: Option<OwnedHandle>,
39+
pub stdout: Option<OwnedHandle>,
40+
pub stderr: Option<OwnedHandle>,
4041
}
4142

4243
impl Stdio {
4344
fn make_input(
4445
spec: crate::InputSpecificationData,
45-
) -> Result<(HANDLE, Option<WritePipe>), Error> {
46+
) -> Result<(Option<OwnedHandle>, Option<WritePipe>), Error> {
4647
match spec {
47-
InputSpecificationData::Handle(h) => Ok((h as HANDLE, None)),
48+
InputSpecificationData::Handle(h) => Ok((Some(OwnedHandle::new(h as HANDLE)), None)),
4849
InputSpecificationData::Pipe => {
4950
let (reader, writer) = pipe::make(pipe::InheritKind::Allow)?;
50-
Ok((reader.into_raw_handle(), Some(writer)))
51+
Ok((
52+
Some(OwnedHandle::new(reader.into_raw_handle())),
53+
Some(writer),
54+
))
5155
}
52-
InputSpecificationData::Empty => Ok((open_empty_readable_file(), None)),
53-
InputSpecificationData::Null => Ok((-1_i32 as usize as HANDLE, None)),
56+
InputSpecificationData::Empty => Ok((Some(open_empty_readable_file()), None)),
57+
InputSpecificationData::Null => Ok((None, None)),
5458
}
5559
}
5660

5761
fn make_output(
5862
spec: crate::OutputSpecificationData,
59-
) -> Result<(HANDLE, Option<ReadPipe>), Error> {
63+
) -> Result<(Option<OwnedHandle>, Option<ReadPipe>), Error> {
6064
match spec {
61-
OutputSpecificationData::Handle(h) => Ok((h as HANDLE, None)),
65+
OutputSpecificationData::Handle(h) => Ok((Some(OwnedHandle::new(h as HANDLE)), None)),
6266
OutputSpecificationData::Pipe => {
6367
let (reader, writer) = pipe::make(pipe::InheritKind::Allow)?;
64-
Ok((writer.into_raw_handle(), Some(reader)))
68+
Ok((
69+
Some(OwnedHandle::new(writer.into_raw_handle())),
70+
Some(reader),
71+
))
6572
}
66-
OutputSpecificationData::Null => Ok((-1_i32 as usize as HANDLE, None)),
73+
OutputSpecificationData::Null => Ok((None, None)),
6774
OutputSpecificationData::Ignore => {
6875
let file = std::fs::File::create("C:\\NUL").map_err(|io_err| Error::Syscall {
6976
errno: io_err.raw_os_error().unwrap_or(-1) as u32,
7077
})?;
71-
let file = file.into_raw_handle();
72-
let cloned_file = crate::windows::util::duplicate_with_inheritance(file)?;
73-
unsafe {
74-
CloseHandle(file);
75-
}
76-
Ok((cloned_file, None))
78+
let file = OwnedHandle::new(file.into_raw_handle());
79+
let cloned_file = file.try_clone_with_inheritance()?;
80+
Ok((Some(cloned_file), None))
7781
}
7882
OutputSpecificationData::Buffer(sz) => unsafe {
7983
let sz = sz.unwrap_or(usize::max_value()) as u64;
@@ -88,8 +92,12 @@ impl Stdio {
8892
if mmap.is_null() {
8993
Cvt::nonzero(0)?;
9094
}
91-
let child_side = crate::windows::util::duplicate_with_inheritance(mmap)?;
92-
Ok((child_side, Some(ReadPipe::from_raw_handle(mmap))))
95+
let mmap = OwnedHandle::new(mmap);
96+
let child_side = mmap.try_clone_with_inheritance()?;
97+
Ok((
98+
Some(child_side),
99+
Some(ReadPipe::from_raw_handle(mmap.into_inner())),
100+
))
93101
},
94102
}
95103
}
@@ -117,14 +125,17 @@ impl Stdio {
117125
}
118126
}
119127

120-
fn open_empty_readable_file() -> HANDLE {
121-
static EMPTY_FILE_HANDLE: once_cell::sync::Lazy<usize> = once_cell::sync::Lazy::new(|| {
122-
let (reader, writer) =
123-
pipe::make(pipe::InheritKind::Allow).expect("failed to create a pipe");
124-
drop(writer);
125-
reader.into_raw_handle() as usize
126-
});
127-
*EMPTY_FILE_HANDLE as HANDLE
128+
fn open_empty_readable_file() -> OwnedHandle {
129+
static EMPTY_FILE_HANDLE: once_cell::sync::Lazy<OwnedHandle> =
130+
once_cell::sync::Lazy::new(|| {
131+
let (reader, writer) =
132+
pipe::make(pipe::InheritKind::Allow).expect("failed to create a pipe");
133+
drop(writer);
134+
OwnedHandle::new(reader.into_raw_handle())
135+
});
136+
EMPTY_FILE_HANDLE
137+
.try_clone()
138+
.expect("failed to clone a handle")
128139
}
129140

130141
pub(in crate::windows) struct ChildParams {
@@ -211,9 +222,15 @@ pub(in crate::windows) fn spawn(
211222

212223
startup_info.StartupInfo.cb = size_of::<STARTUPINFOEXW>() as u32;
213224
startup_info.StartupInfo.dwFlags = STARTF_USESTDHANDLES;
214-
startup_info.StartupInfo.hStdInput = stdio.stdin;
215-
startup_info.StartupInfo.hStdOutput = stdio.stdout;
216-
startup_info.StartupInfo.hStdError = stdio.stderr;
225+
226+
let cvt_handle = |h: Option<OwnedHandle>| {
227+
h.map(OwnedHandle::into_inner)
228+
.unwrap_or(INVALID_HANDLE_VALUE)
229+
};
230+
231+
startup_info.StartupInfo.hStdInput = cvt_handle(stdio.stdin);
232+
startup_info.StartupInfo.hStdOutput = cvt_handle(stdio.stdout);
233+
startup_info.StartupInfo.hStdError = cvt_handle(stdio.stderr);
217234
startup_info
218235
};
219236
let creation_flags = CREATE_UNICODE_ENVIRONMENT | EXTENDED_STARTUPINFO_PRESENT;

0 commit comments

Comments
 (0)