Skip to content

Commit 5432732

Browse files
Yury Samkevichfacebook-github-bot
Yury Samkevich
authored andcommitted
remove iteration over threads on Windows
Summary: Change `resume_process` implementation to get main thread handle through [unstable api](rust-lang/rust#96723) in std::process::Child and not use `CreateToolhelp32Snapshot`, because iteration over treads cause performance issue. Reviewed By: stepancheg Differential Revision: D53472748 fbshipit-source-id: 2589ade412363cf96f5fcb2f896517d4650d0235
1 parent ea0a5f1 commit 5432732

File tree

2 files changed

+14
-62
lines changed

2 files changed

+14
-62
lines changed

app/buck2_forkserver/src/lib.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99

1010
#![feature(error_generic_member_access)]
1111
#![feature(offset_of)]
12+
#![cfg_attr(windows, feature(windows_process_extensions_main_thread_handle))]
1213
pub mod client;
1314
pub mod convert;
1415
pub mod run;

app/buck2_forkserver/src/win/process_group.rs

Lines changed: 13 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -8,22 +8,19 @@
88
*/
99

1010
use std::os::windows::io::AsRawHandle;
11+
use std::os::windows::process::ChildExt;
1112
use std::os::windows::process::CommandExt;
1213
use std::process::Child;
1314
use std::process::Command;
1415
use std::process::ExitStatus;
1516
use std::process::Stdio;
1617

1718
use buck2_error::Context;
18-
use buck2_wrapper_common::winapi_handle::WinapiHandle;
1919
use tokio::io;
2020
use tokio::process::ChildStderr;
2121
use tokio::process::ChildStdout;
2222
use winapi::shared::minwindef;
23-
use winapi::um::handleapi;
2423
use winapi::um::processthreadsapi;
25-
use winapi::um::tlhelp32;
26-
use winapi::um::winnt;
2724

2825
use crate::win::child_process::ChildProcess;
2926
use crate::win::job_object::JobObject;
@@ -143,66 +140,20 @@ impl ProcessGroupImpl {
143140
}
144141

145142
fn resume(&self) -> anyhow::Result<()> {
146-
let process_id = self.id().context("can't resume an exited process")?;
143+
let handle = self
144+
.child
145+
.as_option()
146+
.context("can't resume an exited process")?
147+
.as_std()
148+
.main_thread_handle()
149+
.as_raw_handle();
147150
unsafe {
148-
let main_thread_id = get_main_thread(process_id)?;
149-
resume_thread(main_thread_id)
150-
}
151-
}
152-
}
153-
154-
// We need main thread handle to resume process after assigning it to JobObject
155-
// Currently there is no way to get main thread handle from tokio's process::Child
156-
// We use CreateToolhelp32Snapshot to get a snapshot of all threads in the system
157-
// and find the one with the given process_id.
158-
// todo(yurysamkevich): use main_thread_handle once issue is closed
159-
// https://github.com/tokio-rs/tokio/issues/6153
160-
unsafe fn get_main_thread(process_id: u32) -> anyhow::Result<minwindef::DWORD> {
161-
let snapshot_handle = tlhelp32::CreateToolhelp32Snapshot(tlhelp32::TH32CS_SNAPTHREAD, 0);
162-
if snapshot_handle == handleapi::INVALID_HANDLE_VALUE {
163-
return Err(anyhow::anyhow!("Failed to list threads"));
164-
}
165-
let snapshot_handle = WinapiHandle::new(snapshot_handle);
166-
let mut thread_entry_32 = tlhelp32::THREADENTRY32 {
167-
dwSize: std::mem::size_of::<tlhelp32::THREADENTRY32>() as u32,
168-
cntUsage: 0,
169-
th32ThreadID: 0,
170-
th32OwnerProcessID: 0,
171-
tpBasePri: 0,
172-
tpDeltaPri: 0,
173-
dwFlags: 0,
174-
};
175-
let raw_pointer_to_thread_entry_32 = &mut thread_entry_32 as *mut tlhelp32::THREADENTRY32;
176-
177-
let mut thread_result =
178-
tlhelp32::Thread32First(snapshot_handle.handle(), raw_pointer_to_thread_entry_32);
179-
while thread_result == minwindef::TRUE {
180-
if thread_entry_32.dwSize as usize
181-
>= std::mem::offset_of!(tlhelp32::THREADENTRY32, th32OwnerProcessID)
182-
{
183-
if thread_entry_32.th32OwnerProcessID == process_id {
184-
return Ok(thread_entry_32.th32ThreadID);
151+
let resume_thread_result = processthreadsapi::ResumeThread(handle);
152+
if resume_thread_result == minwindef::DWORD::MAX {
153+
Err(anyhow::anyhow!("Failed to resume thread"))
154+
} else {
155+
Ok(())
185156
}
186157
}
187-
thread_result = winapi::um::tlhelp32::Thread32Next(
188-
snapshot_handle.handle(),
189-
raw_pointer_to_thread_entry_32,
190-
);
191-
}
192-
Err(anyhow::anyhow!("Failed to find thread to resume"))
193-
}
194-
195-
unsafe fn resume_thread(thread_id: minwindef::DWORD) -> anyhow::Result<()> {
196-
let thread_handle =
197-
processthreadsapi::OpenThread(winnt::THREAD_SUSPEND_RESUME, minwindef::FALSE, thread_id);
198-
if thread_handle.is_null() {
199-
return Err(anyhow::anyhow!("Failed to open thread to resume"));
200-
}
201-
let thread_handle = WinapiHandle::new(thread_handle);
202-
let resume_thread_result = processthreadsapi::ResumeThread(thread_handle.handle());
203-
if resume_thread_result == minwindef::DWORD::MAX {
204-
Err(anyhow::anyhow!("Failed to resume thread"))
205-
} else {
206-
Ok(())
207158
}
208159
}

0 commit comments

Comments
 (0)