Skip to content

Commit e06641d

Browse files
coolreader18coolreader18
committed
Fix all the weird Windows bugs
1 parent a7131ee commit e06641d

5 files changed

Lines changed: 55 additions & 25 deletions

File tree

‎Lib/subprocess.py‎

Lines changed: 20 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -807,13 +807,14 @@ def __init__(self, args, bufsize=-1, executable=None,
807807
# quickly terminating child could make our fds unwrappable
808808
# (see #8458).
809809

810-
if _mswindows:
811-
if p2cwrite != -1:
812-
p2cwrite = msvcrt.open_osfhandle(p2cwrite.Detach(), 0)
813-
if c2pread != -1:
814-
c2pread = msvcrt.open_osfhandle(c2pread.Detach(), 0)
815-
if errread != -1:
816-
errread = msvcrt.open_osfhandle(errread.Detach(), 0)
810+
# XXX RustPython TODO: have fds for fs functions be actual CRT fds on windows, not handles
811+
# if _mswindows:
812+
# if p2cwrite != -1:
813+
# p2cwrite = msvcrt.open_osfhandle(p2cwrite.Detach(), 0)
814+
# if c2pread != -1:
815+
# c2pread = msvcrt.open_osfhandle(c2pread.Detach(), 0)
816+
# if errread != -1:
817+
# errread = msvcrt.open_osfhandle(errread.Detach(), 0)
817818

818819
self.text_mode = encoding or errors or text or universal_newlines
819820

@@ -1154,7 +1155,11 @@ def _get_handles(self, stdin, stdout, stderr):
11541155
else:
11551156
# Assuming file-like object
11561157
p2cread = msvcrt.get_osfhandle(stdin.fileno())
1158+
# XXX RUSTPYTHON TODO: figure out why closing these old, non-inheritable
1159+
# pipe handles is necessary for us, but not CPython
1160+
old = p2cread
11571161
p2cread = self._make_inheritable(p2cread)
1162+
if stdin == PIPE: _winapi.CloseHandle(old)
11581163

11591164
if stdout is None:
11601165
c2pwrite = _winapi.GetStdHandle(_winapi.STD_OUTPUT_HANDLE)
@@ -1172,7 +1177,11 @@ def _get_handles(self, stdin, stdout, stderr):
11721177
else:
11731178
# Assuming file-like object
11741179
c2pwrite = msvcrt.get_osfhandle(stdout.fileno())
1180+
# XXX RUSTPYTHON TODO: figure out why closing these old, non-inheritable
1181+
# pipe handles is necessary for us, but not CPython
1182+
old = c2pwrite
11751183
c2pwrite = self._make_inheritable(c2pwrite)
1184+
if stdout == PIPE: _winapi.CloseHandle(old)
11761185

11771186
if stderr is None:
11781187
errwrite = _winapi.GetStdHandle(_winapi.STD_ERROR_HANDLE)
@@ -1192,7 +1201,11 @@ def _get_handles(self, stdin, stdout, stderr):
11921201
else:
11931202
# Assuming file-like object
11941203
errwrite = msvcrt.get_osfhandle(stderr.fileno())
1204+
# XXX RUSTPYTHON TODO: figure out why closing these old, non-inheritable
1205+
# pipe handles is necessary for us, but not CPython
1206+
old = errwrite
11951207
errwrite = self._make_inheritable(errwrite)
1208+
if stderr == PIPE: _winapi.CloseHandle(old)
11961209

11971210
return (p2cread, p2cwrite,
11981211
c2pread, c2pwrite,

‎tests/snippets/stdlib_subprocess.py‎

Lines changed: 20 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,20 @@
55

66
from testutils import assert_raises
77

8-
p = subprocess.Popen(["echo", "test"])
8+
is_unix = not sys.platform.startswith("win")
9+
if is_unix:
10+
def echo(text):
11+
return ["echo", text]
12+
def sleep(secs):
13+
return ["sleep", str(secs)]
14+
else:
15+
def echo(text):
16+
return ["cmd", "/C", f"echo {text}"]
17+
def sleep(secs):
18+
# TODO: make work in a non-unixy environment (something with timeout.exe?)
19+
return ["sleep", str(secs)]
20+
21+
p = subprocess.Popen(echo("test"))
922

1023
time.sleep(0.1)
1124

@@ -14,7 +27,7 @@
1427
assert p.poll() == 0
1528
assert p.returncode == 0
1629

17-
p = subprocess.Popen(["sleep", "2"])
30+
p = subprocess.Popen(sleep(2))
1831

1932
assert p.poll() is None
2033

@@ -25,33 +38,32 @@
2538

2639
assert p.returncode == 0
2740

28-
p = subprocess.Popen(["echo", "test"], stdout=subprocess.PIPE)
41+
p = subprocess.Popen(echo("test"), stdout=subprocess.PIPE)
2942
p.wait()
3043

31-
is_unix = "win" not in sys.platform or "darwin" in sys.platform
3244

3345
assert p.stdout.read().strip() == b"test"
3446

35-
p = subprocess.Popen(["sleep", "2"])
47+
p = subprocess.Popen(sleep(2))
3648
p.terminate()
3749
p.wait()
3850
if is_unix:
3951
assert p.returncode == -signal.SIGTERM
4052
else:
4153
assert p.returncode == 1
4254

43-
p = subprocess.Popen(["sleep", "2"])
55+
p = subprocess.Popen(sleep(2))
4456
p.kill()
4557
p.wait()
4658
if is_unix:
4759
assert p.returncode == -signal.SIGKILL
4860
else:
4961
assert p.returncode == 1
5062

51-
p = subprocess.Popen(["echo", "test"], stdout=subprocess.PIPE)
63+
p = subprocess.Popen(echo("test"), stdout=subprocess.PIPE)
5264
(stdout, stderr) = p.communicate()
5365
assert stdout.strip() == b"test"
5466

55-
p = subprocess.Popen(["sleep", "5"], stdout=subprocess.PIPE)
67+
p = subprocess.Popen(sleep(5), stdout=subprocess.PIPE)
5668
with assert_raises(subprocess.TimeoutExpired):
5769
p.communicate(timeout=1)

‎vm/src/stdlib/os.rs‎

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ use crate::exceptions::PyBaseExceptionRef;
2828
use crate::function::{IntoPyNativeFunc, OptionalArg, PyFuncArgs};
2929
use crate::obj::objbyteinner::PyBytesLike;
3030
use crate::obj::objbytes::{PyBytes, PyBytesRef};
31-
use crate::obj::objdict::{PyDictRef, PyMapping};
31+
use crate::obj::objdict::PyDictRef;
3232
use crate::obj::objint::PyIntRef;
3333
use crate::obj::objiter;
3434
use crate::obj::objset::PySet;
@@ -43,6 +43,8 @@ use crate::vm::VirtualMachine;
4343

4444
// just to avoid unused import warnings
4545
#[cfg(unix)]
46+
use crate::obj::objdict::PyMapping;
47+
#[cfg(unix)]
4648
use crate::pyobject::PyIterable;
4749
#[cfg(unix)]
4850
use std::convert::TryFrom;
@@ -124,7 +126,7 @@ fn output_by_mode(val: String, mode: OutputMode, vm: &VirtualMachine) -> PyObjec
124126
}
125127

126128
pub struct PyPathLike {
127-
path: ffi::OsString,
129+
pub path: ffi::OsString,
128130
mode: OutputMode,
129131
}
130132

@@ -140,9 +142,6 @@ impl PyPathLike {
140142
use std::os::windows::ffi::OsStrExt;
141143
self.path.encode_wide().chain(std::iter::once(0)).collect()
142144
}
143-
pub fn into_os_string(self) -> ffi::OsString {
144-
self.path
145-
}
146145
}
147146

148147
impl TryFromObject for PyPathLike {

‎vm/src/stdlib/posixsubprocess.rs‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -23,9 +23,7 @@ struct CStrPathLike {
2323
}
2424
impl TryFromObject for CStrPathLike {
2525
fn try_from_object(vm: &VirtualMachine, obj: PyObjectRef) -> PyResult<Self> {
26-
let s = PyPathLike::try_from_object(vm, obj)?
27-
.into_os_string()
28-
.into_vec();
26+
let s = PyPathLike::try_from_object(vm, obj)?.path.into_vec();
2927
let s = CString::new(s)
3028
.map_err(|_| vm.new_value_error("embedded null character".to_owned()))?;
3129
Ok(CStrPathLike { s })

‎vm/src/stdlib/winapi.rs‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -132,7 +132,7 @@ fn _winapi_CreateProcess(
132132
) -> PyResult<(usize, usize, u32, u32)> {
133133
use winbase::STARTUPINFOEXW;
134134
let mut si: STARTUPINFOEXW = unsafe { std::mem::zeroed() };
135-
si.StartupInfo.cb = 84; // std::mem::size_of::<STARTUPINFOEXW>() as _;
135+
si.StartupInfo.cb = std::mem::size_of::<STARTUPINFOEXW>() as _;
136136

137137
macro_rules! si_attr {
138138
($attr:ident, $t:ty) => {{
@@ -309,7 +309,7 @@ fn getattributelist(obj: PyObjectRef, vm: &VirtualMachine) -> PyResult<Option<At
309309
0,
310310
(2 & 0xffff) | 0x20000, // PROC_THREAD_ATTRIBUTE_HANDLE_LIST
311311
handlelist.as_mut_ptr() as _,
312-
handlelist.len() as _,
312+
(handlelist.len() * std::mem::size_of::<HANDLE>()) as _,
313313
null_mut(),
314314
null_mut(),
315315
)
@@ -340,6 +340,13 @@ fn _winapi_GetExitCodeProcess(h: usize, vm: &VirtualMachine) -> PyResult<u32> {
340340
Ok(ec)
341341
}
342342

343+
fn _winapi_TerminateProcess(h: usize, exit_code: u32, vm: &VirtualMachine) -> PyResult<()> {
344+
cvt(vm, unsafe {
345+
processthreadsapi::TerminateProcess(h as _, exit_code)
346+
})
347+
.map(drop)
348+
}
349+
343350
pub fn make_module(vm: &VirtualMachine) -> PyObjectRef {
344351
let ctx = &vm.ctx;
345352
py_module!(vm, "_winapi", {
@@ -351,6 +358,7 @@ pub fn make_module(vm: &VirtualMachine) -> PyObjectRef {
351358
"CreateProcess" => named_function!(ctx, _winapi, CreateProcess),
352359
"WaitForSingleObject" => named_function!(ctx, _winapi, WaitForSingleObject),
353360
"GetExitCodeProcess" => named_function!(ctx, _winapi, GetExitCodeProcess),
361+
"TerminateProcess" => named_function!(ctx, _winapi, TerminateProcess),
354362

355363
"WAIT_OBJECT_0" => ctx.new_int(winbase::WAIT_OBJECT_0),
356364
"WAIT_ABANDONED" => ctx.new_int(winbase::WAIT_ABANDONED),

0 commit comments

Comments
 (0)