Skip to content

Commit fe3b51e

Browse files
youknowoneyouknowone
committed
apply code review
1 parent 9f76666 commit fe3b51e

5 files changed

Lines changed: 58 additions & 33 deletions

File tree

‎crates/codegen/src/ir.rs‎

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3309,7 +3309,6 @@ fn is_scope_exit_block(block: &Block) -> bool {
33093309
.is_some_and(|instr| instr.instr.is_scope_exit())
33103310
}
33113311

3312-
#[allow(dead_code)]
33133312
fn is_exception_cleanup_block(block: &Block) -> bool {
33143313
block
33153314
.instructions
@@ -3322,7 +3321,7 @@ fn is_exception_cleanup_block(block: &Block) -> bool {
33223321
}
33233322

33243323
fn block_is_exceptional(block: &Block) -> bool {
3325-
block.except_handler || block.preserve_lasti
3324+
block.except_handler || block.preserve_lasti || is_exception_cleanup_block(block)
33263325
}
33273326

33283327
fn trailing_conditional_jump_index(block: &Block) -> Option<usize> {
@@ -3773,9 +3772,6 @@ fn duplicate_jump_targets_without_lineno(blocks: &mut Vec<Block>, predecessors:
37733772
last_mut.target = new_idx;
37743773
predecessors[target.idx()] -= 1;
37753774
predecessors.push(1);
3776-
if old_next != BlockIdx::NULL {
3777-
predecessors[old_next.idx()] += 1;
3778-
}
37793775

37803776
current = old_next;
37813777
}

‎crates/vm/src/stdlib/_thread.rs‎

Lines changed: 12 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -55,8 +55,6 @@ pub(crate) mod _thread {
5555
// this is a value in seconds
5656
#[pyattr]
5757
const TIMEOUT_MAX: f64 = (TIMEOUT_MAX_IN_MICROSECONDS / 1_000_000) as f64;
58-
const DEFAULT_THREAD_STACK_SIZE: usize = 8 * 1024 * 1024;
59-
6058
#[pyattr]
6159
fn error(vm: &VirtualMachine) -> PyTypeRef {
6260
vm.ctx.exceptions.runtime_error.to_owned()
@@ -595,12 +593,11 @@ pub(crate) mod _thread {
595593
vm: &VirtualMachine,
596594
) -> thread::Builder {
597595
let configured = vm.state.stacksize.load();
598-
let stack_size = if configured != 0 {
599-
configured
596+
if configured != 0 {
597+
thread_builder.stack_size(configured)
600598
} else {
601-
VirtualMachine::current_c_stack_size().max(DEFAULT_THREAD_STACK_SIZE)
602-
};
603-
thread_builder.stack_size(stack_size)
599+
thread_builder
600+
}
604601
}
605602

606603
/// Clean up thread-local data for the current thread.
@@ -903,7 +900,7 @@ pub(crate) mod _thread {
903900
// Guard that removes thread-local data when dropped
904901
struct LocalGuard {
905902
local: Weak<LocalData>,
906-
thread_id: std::thread::ThreadId,
903+
thread_id: u64,
907904
}
908905

909906
impl Drop for LocalGuard {
@@ -919,7 +916,7 @@ pub(crate) mod _thread {
919916

920917
// Shared data structure for Local
921918
struct LocalData {
922-
data: parking_lot::Mutex<std::collections::HashMap<std::thread::ThreadId, PyDictRef>>,
919+
data: parking_lot::Mutex<std::collections::HashMap<u64, PyDictRef>>,
923920
}
924921

925922
impl fmt::Debug for LocalData {
@@ -938,7 +935,7 @@ pub(crate) mod _thread {
938935
#[pyclass(with(GetAttr, SetAttr), flags(BASETYPE))]
939936
impl Local {
940937
fn l_dict(&self, vm: &VirtualMachine) -> PyDictRef {
941-
let thread_id = std::thread::current().id();
938+
let thread_id = current_thread_id();
942939

943940
// Fast path: check if dict exists under lock
944941
if let Some(dict) = self.inner.data.lock().get(&thread_id).cloned() {
@@ -974,6 +971,11 @@ pub(crate) mod _thread {
974971
dict
975972
}
976973

974+
#[pygetset(name = "__dict__")]
975+
fn dict(zelf: PyRef<Self>, vm: &VirtualMachine) -> PyDictRef {
976+
zelf.l_dict(vm)
977+
}
978+
977979
#[pyslot]
978980
fn slot_new(cls: PyTypeRef, _args: FuncArgs, vm: &VirtualMachine) -> PyResult {
979981
Self {

‎crates/vm/src/vm/mod.rs‎

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1523,17 +1523,6 @@ impl VirtualMachine {
15231523
0
15241524
}
15251525

1526-
#[cfg(all(feature = "threading", not(miri), not(target_env = "musl")))]
1527-
pub(crate) fn current_c_stack_size() -> usize {
1528-
let (base, top) = Self::get_stack_bounds();
1529-
top.saturating_sub(base)
1530-
}
1531-
1532-
#[cfg(all(feature = "threading", any(miri, target_env = "musl")))]
1533-
pub(crate) fn current_c_stack_size() -> usize {
1534-
8 * 1024 * 1024
1535-
}
1536-
15371526
/// Check if we're near the C stack limit (like _Py_MakeRecCheck).
15381527
/// Returns true only when stack pointer is in the "danger zone" between
15391528
/// soft_limit and hard_limit (soft_limit - 2*margin).

‎crates/vm/src/vm/thread.rs‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -557,6 +557,10 @@ impl ThreadedVirtualMachine {
557557
F: FnOnce(&VirtualMachine) -> R,
558558
{
559559
let vm = &self.vm;
560+
// Each spawned thread has its own native stack bounds. Recompute the
561+
// soft limit here instead of inheriting the parent thread's value.
562+
vm.c_stack_soft_limit
563+
.set(VirtualMachine::calculate_c_stack_soft_limit());
560564
enter_vm(vm, || f(vm))
561565
}
562566
}

‎extra_tests/snippets/stdlib_threading.py‎

Lines changed: 41 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,37 +1,71 @@
1+
import multiprocessing
2+
import os
13
import threading
24

35

46
def import_in_thread(module_name):
57
outcome = {}
8+
error = {}
69

710
def worker():
8-
module = __import__(module_name, fromlist=["*"])
9-
outcome["name"] = module.__name__
11+
try:
12+
module = __import__(module_name, fromlist=["*"])
13+
outcome["name"] = module.__name__
14+
except Exception as exc:
15+
error["exc"] = exc
1016

1117
thread = threading.Thread(target=worker)
1218
thread.start()
13-
thread.join()
19+
thread.join(timeout=5)
20+
assert not thread.is_alive(), "thread did not finish in time"
21+
if "exc" in error:
22+
raise error["exc"]
1423

1524
assert outcome["name"] == module_name
1625

1726

1827
def run_exec(code):
1928
result = {}
29+
error = {}
2030

2131
def worker():
22-
scope = {"__builtins__": __builtins__}
23-
exec(code, scope, scope)
24-
result["scope"] = scope
32+
try:
33+
scope = {"__builtins__": __builtins__}
34+
exec(code, scope, scope) # noqa: S102 - intentional threaded exec regression test
35+
result["scope"] = scope
36+
except Exception as exc:
37+
error["exc"] = exc
2538

2639
thread = threading.Thread(target=worker)
2740
thread.start()
28-
thread.join()
41+
thread.join(timeout=5)
42+
assert not thread.is_alive(), "thread did not finish in time"
43+
if "exc" in error:
44+
raise error["exc"]
2945
return result["scope"]
3046

3147

48+
def child_process():
49+
return None
50+
51+
52+
def start_fork_process_after_thread():
53+
if not hasattr(os, "fork"):
54+
return
55+
56+
import_in_thread("multiprocessing.connection")
57+
58+
ctx = multiprocessing.get_context("fork")
59+
process = ctx.Process(target=child_process)
60+
process.start()
61+
process.join(timeout=10)
62+
assert process.exitcode == 0, process.exitcode
63+
64+
3265
import_in_thread("functools")
3366
import_in_thread("tempfile")
3467
import_in_thread("multiprocessing.connection")
68+
start_fork_process_after_thread()
3569

3670
scope = run_exec("import functools")
3771
assert scope["functools"].__name__ == "functools"

0 commit comments

Comments
 (0)