Skip to content

Allow heap getset creation #5854

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jun 28, 2025
Merged

Allow heap getset creation #5854

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
04cc124
Update _threading_local.py
youknowone Jun 28, 2025
f57cddf
allow heap-getset
youknowone Jun 27, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
131 changes: 1 addition & 130 deletions Lib/_threading_local.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,133 +4,6 @@
class. Depending on the version of Python you're using, there may be a
faster one available. You should always import the `local` class from
`threading`.)

Thread-local objects support the management of thread-local data.
If you have data that you want to be local to a thread, simply create
a thread-local object and use its attributes:

>>> mydata = local()
>>> mydata.number = 42
>>> mydata.number
42

You can also access the local-object's dictionary:

>>> mydata.__dict__
{'number': 42}
>>> mydata.__dict__.setdefault('widgets', [])
[]
>>> mydata.widgets
[]

What's important about thread-local objects is that their data are
local to a thread. If we access the data in a different thread:

>>> log = []
>>> def f():
... items = sorted(mydata.__dict__.items())
... log.append(items)
... mydata.number = 11
... log.append(mydata.number)

>>> import threading
>>> thread = threading.Thread(target=f)
>>> thread.start()
>>> thread.join()
>>> log
[[], 11]

we get different data. Furthermore, changes made in the other thread
don't affect data seen in this thread:

>>> mydata.number
42

Of course, values you get from a local object, including a __dict__
attribute, are for whatever thread was current at the time the
attribute was read. For that reason, you generally don't want to save
these values across threads, as they apply only to the thread they
came from.

You can create custom local objects by subclassing the local class:

>>> class MyLocal(local):
... number = 2
... initialized = False
... def __init__(self, **kw):
... if self.initialized:
... raise SystemError('__init__ called too many times')
... self.initialized = True
... self.__dict__.update(kw)
... def squared(self):
... return self.number ** 2

This can be useful to support default values, methods and
initialization. Note that if you define an __init__ method, it will be
called each time the local object is used in a separate thread. This
is necessary to initialize each thread's dictionary.

Now if we create a local object:

>>> mydata = MyLocal(color='red')

Now we have a default number:

>>> mydata.number
2

an initial color:

>>> mydata.color
'red'
>>> del mydata.color

And a method that operates on the data:

>>> mydata.squared()
4

As before, we can access the data in a separate thread:

>>> log = []
>>> thread = threading.Thread(target=f)
>>> thread.start()
>>> thread.join()
>>> log
[[('color', 'red'), ('initialized', True)], 11]

without affecting this thread's data:

>>> mydata.number
2
>>> mydata.color
Traceback (most recent call last):
...
AttributeError: 'MyLocal' object has no attribute 'color'

Note that subclasses can define slots, but they are not thread
local. They are shared across threads:

>>> class MyLocal(local):
... __slots__ = 'number'

>>> mydata = MyLocal()
>>> mydata.number = 42
>>> mydata.color = 'red'

So, the separate thread:

>>> thread = threading.Thread(target=f)
>>> thread.start()
>>> thread.join()

affects what we see:

>>> # TODO: RUSTPYTHON, __slots__
>>> mydata.number #doctest: +SKIP
11

>>> del mydata
"""

from weakref import ref
Expand Down Expand Up @@ -194,7 +67,6 @@ def thread_deleted(_, idt=idt):

@contextmanager
def _patch(self):
old = object.__getattribute__(self, '__dict__')
impl = object.__getattribute__(self, '_local__impl')
try:
dct = impl.get_dict()
Expand All @@ -205,13 +77,12 @@ def _patch(self):
with impl.locallock:
object.__setattr__(self, '__dict__', dct)
yield
object.__setattr__(self, '__dict__', old)


class local:
__slots__ = '_local__impl', '__dict__'

def __new__(cls, *args, **kw):
def __new__(cls, /, *args, **kw):
if (args or kw) and (cls.__init__ is object.__init__):
raise TypeError("Initialization arguments are not supported")
self = object.__new__(cls)
Expand Down
2 changes: 1 addition & 1 deletion stdlib/src/pyexpat.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ pub fn make_module(vm: &VirtualMachine) -> PyRef<PyModule> {

macro_rules! create_property {
($ctx: expr, $attributes: expr, $name: expr, $class: expr, $element: ident) => {
let attr = $ctx.new_getset(
let attr = $ctx.new_static_getset(
$name,
$class,
move |this: &PyExpatLikeXmlParser| this.$element.read().clone(),
Expand Down
13 changes: 9 additions & 4 deletions vm/src/builtins/getset.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
use super::PyType;
use crate::{
AsObject, Context, Py, PyObject, PyObjectRef, PyPayload, PyResult, VirtualMachine,
builtins::type_::PointerSlot,
class::PyClassImpl,
function::{IntoPyGetterFunc, IntoPySetterFunc, PyGetterFunc, PySetterFunc, PySetterValue},
types::{GetDescriptor, Unconstructible},
Expand All @@ -12,7 +13,7 @@ use crate::{
#[pyclass(module = false, name = "getset_descriptor")]
pub struct PyGetSet {
name: String,
class: &'static Py<PyType>,
class: PointerSlot<Py<PyType>>, // A class type freed before getset is non-sense.
getter: Option<PyGetterFunc>,
setter: Option<PySetterFunc>,
// doc: Option<String>,
Expand Down Expand Up @@ -72,7 +73,7 @@ impl PyGetSet {
pub fn new(name: String, class: &'static Py<PyType>) -> Self {
Self {
name,
class,
class: PointerSlot::from(class),
getter: None,
setter: None,
}
Expand Down Expand Up @@ -138,13 +139,17 @@ impl PyGetSet {

#[pygetset]
fn __qualname__(&self) -> String {
format!("{}.{}", self.class.slot_name(), self.name.clone())
format!(
"{}.{}",
unsafe { self.class.borrow_static() }.slot_name(),
self.name.clone()
)
}

#[pymember]
fn __objclass__(vm: &VirtualMachine, zelf: PyObjectRef) -> PyResult {
let zelf: &Py<Self> = zelf.try_to_value(vm)?;
Ok(zelf.class.to_owned().into())
Ok(unsafe { zelf.class.borrow_static() }.to_owned().into())
}
}
impl Unconstructible for PyGetSet {}
Expand Down
2 changes: 1 addition & 1 deletion vm/src/builtins/property.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -386,7 +386,7 @@ pub(crate) fn init(context: &Context) {
// This is a bit unfortunate, but this instance attribute overlaps with the
// class __doc__ string..
extend_class!(context, context.types.property_type, {
"__doc__" => context.new_getset(
"__doc__" => context.new_static_getset(
"__doc__",
context.types.property_type,
PyProperty::doc_getter,
Expand Down
17 changes: 10 additions & 7 deletions vm/src/builtins/type.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ use crate::{
};
use indexmap::{IndexMap, map::Entry};
use itertools::Itertools;
use std::{borrow::Borrow, collections::HashSet, fmt, ops::Deref, pin::Pin, ptr::NonNull};
use std::{borrow::Borrow, collections::HashSet, ops::Deref, pin::Pin, ptr::NonNull};

#[pyclass(module = false, name = "type", traverse = "manual")]
pub struct PyType {
Expand Down Expand Up @@ -69,6 +69,9 @@ pub struct HeapTypeExt {

pub struct PointerSlot<T>(NonNull<T>);

unsafe impl<T> Sync for PointerSlot<T> {}
unsafe impl<T> Send for PointerSlot<T> {}
Comment on lines +72 to +73
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Critical: Overly broad unsafe trait implementations pose safety risks.

These blanket unsafe impl Sync/Send for PointerSlot<T> implementations are concerning because:

  1. They apply to any type T without safety constraints
  2. NonNull<T> alone doesn't guarantee the pointed-to data is thread-safe
  3. This could enable unsound code if PointerSlot is used with non-thread-safe data

Consider constraining these implementations:

-unsafe impl<T> Sync for PointerSlot<T> {}
-unsafe impl<T> Send for PointerSlot<T> {}
+unsafe impl<T: Sync> Sync for PointerSlot<T> {}
+unsafe impl<T: Send> Send for PointerSlot<T> {}

Or if the use case is specifically for static/immutable type data:

-unsafe impl<T> Sync for PointerSlot<T> {}
-unsafe impl<T> Send for PointerSlot<T> {}
+unsafe impl<T: Sync + Send + 'static> Sync for PointerSlot<T> {}
+unsafe impl<T: Sync + Send + 'static> Send for PointerSlot<T> {}
🤖 Prompt for AI Agents 
In vm/src/builtins/type.rs at lines 72-73, the unsafe impls of Sync and Send for
PointerSlot<T> are too broad and unsafe because they apply to all T without
constraints. To fix this, add appropriate trait bounds to ensure T is Sync and
Send before implementing these traits for PointerSlot<T>. This will restrict the
implementations to only thread-safe types and prevent unsound behavior.


impl<T> PointerSlot<T> {
pub unsafe fn borrow_static(&self) -> &'static T {
unsafe { self.0.as_ref() }
Expand Down Expand Up @@ -126,14 +129,14 @@ unsafe impl Traverse for PyAttributes {
}
}

impl fmt::Display for PyType {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
fmt::Display::fmt(&self.name(), f)
impl std::fmt::Display for PyType {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
std::fmt::Display::fmt(&self.name(), f)
}
}

impl fmt::Debug for PyType {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
impl std::fmt::Debug for PyType {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
write!(f, "[PyType {}]", &self.name())
}
}
Expand Down Expand Up @@ -908,7 +911,7 @@ impl Constructor for PyType {
let __dict__ = identifier!(vm, __dict__);
attributes.entry(__dict__).or_insert_with(|| {
vm.ctx
.new_getset(
.new_static_getset(
"__dict__",
vm.ctx.types.type_type,
subtype_get_dict,
Expand Down
2 changes: 1 addition & 1 deletion vm/src/class.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ pub trait PyClassImpl: PyClassDef {
let __dict__ = identifier!(ctx, __dict__);
class.set_attr(
__dict__,
ctx.new_getset(
ctx.new_static_getset(
"__dict__",
class,
crate::builtins::object::object_get_dict,
Expand Down
22 changes: 21 additions & 1 deletion vm/src/vm/context.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -570,7 +570,7 @@ impl Context {
PyRef::new_ref(getset, self.types.getset_type.to_owned(), None)
}

pub fn new_getset<G, S, T, U>(
pub fn new_static_getset<G, S, T, U>(
&self,
name: impl Into<String>,
class: &'static Py<PyType>,
Expand All @@ -586,6 +586,26 @@ impl Context {
PyRef::new_ref(getset, self.types.getset_type.to_owned(), None)
}

/// Creates a new `PyGetSet` with a heap type.
///
/// # Safety
/// In practice, this constructor is safe because a getset is always owned by its `class` type.
/// However, it can be broken if used unconventionally.
pub unsafe fn new_getset<G, S, T, U>(
&self,
name: impl Into<String>,
class: &Py<PyType>,
g: G,
s: S,
) -> PyRef<PyGetSet>
where
G: IntoPyGetterFunc<T>,
S: IntoPySetterFunc<U>,
{
let class = unsafe { &*(class as *const _) };
self.new_static_getset(name, class, g, s)
}

pub fn new_base_object(&self, class: PyTypeRef, dict: Option<PyDictRef>) -> PyObjectRef {
debug_assert_eq!(
class.slots.flags.contains(PyTypeFlags::HAS_DICT),
Expand Down
Loading