Skip to content

Conversation

@corona10
Copy link
Member

@corona10corona10 commented Nov 18, 2023

Copy link
Contributor

@colesburycolesbury left a comment

Choose a reason for hiding this comment

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

Great! Much nicer than manually writing the Py_BEGIN/END_CRITICAL_SECTION

@corona10
Copy link
MemberAuthor

corona10 commented Nov 19, 2023

@colesbury
One thing needs to be discussed:
Do we have to care about the thread-safe for is_dead_weakref?

is_dead_weakref(PyObject*value)
{
if (!PyWeakref_Check(value)){
PyErr_SetString(PyExc_TypeError, "not a weakref");
return-1;
}
intis_dead;
Py_BEGIN_CRITICAL_SECTION(value);
is_dead=_PyWeakref_IS_DEAD(value);
Py_END_CRITICAL_SECTION();
returnis_dead;
}

Or should it be handled from _PyDict_DelItemIf since it is only used for _PyDict_DelItemIf?
if (_PyDict_DelItemIf(dct, key, is_dead_weakref) <0){

/* This function promises that the predicate -> deletion sequence is atomic
* (i.e. protected by the GIL), assuming the predicate itself doesn't
* release the GIL.
*/
int
_PyDict_DelItemIf(PyObject*op, PyObject*key,
int (*predicate)(PyObject*value))
{

If we decide not to handle from is_dead_weakref, we can remove critical section related code fully.

@corona10corona10 merged commit 2bcc0f7 into python:mainNov 19, 2023
@corona10corona10 deleted the gh-112213-weakref branch November 19, 2023 02:07
@colesbury
Copy link
Contributor

@corona10, we'll need to handle the thread-safety issues in _PyWeakref_IS_DEAD().

@corona10
Copy link
MemberAuthor

@corona10, we'll need to handle the thread-safety issues in _PyWeakref_IS_DEAD().

Okay let's handle it with upcoming PR :)

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@corona10@colesbury