-
-
Notifications
You must be signed in to change notification settings - Fork 32.1k
gh-125996: fix thread safety of ordered dict #133734
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.
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
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would you please also provide some timing measurements for basic operations?
Objects/odictobject.c
Outdated
register PyODictObject *od = _PyODictObject_CAST(op); | ||
PyDict_Clear(op); | ||
_odict_clear_nodes(od); | ||
PyDict_Clear((PyObject *)self); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
_PyDict_Clear_LockHeld
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
_PyDict_Clear_LockHeld
expects the critical section of the dict to be held but here in tp_clear it isn't held so critical section held assertions fail, should I also add the critical section to tp_clear?
#ifndef Py_GIL_DISABLED | ||
Py_CLEAR(di->di_odict); | ||
#endif |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we need to hold a lock for the iterator, we can keep the Py_CLEAR(di->di_odict);
#ifndef Py_GIL_DISABLED | ||
Py_CLEAR(di->di_odict); | ||
#endif |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here
Uh oh!
There was an error while loading. Please reload this page.