Skip to content

[WIP] bpo-40533: Make PyObject.ob_refcnt atomic in subinterpreters#19958

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

Closed
vstinner wants to merge 1 commit into from
Closed

[WIP] bpo-40533: Make PyObject.ob_refcnt atomic in subinterpreters #19958

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
7 changes: 7 additions & 0 deletions Include/object.h
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -104,7 +104,14 @@ typedef struct _typeobject PyTypeObject;
*/
typedef struct _object{
_PyObject_HEAD_EXTRA
#ifdef EXPERIMENTAL_ISOLATED_SUBINTERPRETERS
/* bpo-40533: Use an atomic variable for PyObject.ob_refcnt, to ensure that
Py_INCREF() and Py_DECREF() are safe when called in parallel, until
subinterpreters stop sharing Python objects. */
_Atomic Py_ssize_t ob_refcnt;
Comment on lines +107 to +111
Copy link
Contributor

@aerosaerosMay 6, 2020•
edited
Loading

Choose a reason for hiding this comment

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

Perhaps this is somewhat emphasized by the "experimental" part, but could we possibly make it a bit more crystal clear that this is only a temporary stopgap measure? I personally find that "until subinterpreters stop sharing Python objects" could be interpreted as a near indefinite amount of time; I'm not confident that most readers will go over the full context in the bpo issue and related discussions.

Normally, this wouldn't be a huge concern, but I think object.h is likely one of the most public-facing header files in CPython, so it seems important to communicate this as clearly as possible.

#else
Py_ssize_t ob_refcnt;
#endif
PyTypeObject *ob_type;
} PyObject;

Expand Down
6 changes: 6 additions & 0 deletions Objects/dict-common.h
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -20,7 +20,13 @@ typedef Py_ssize_t (*dict_lookup_func)

/* See dictobject.c for actual layout of DictKeysObject */
struct _dictkeysobject{
#ifdef EXPERIMENTAL_ISOLATED_SUBINTERPRETERS
/* bpo-40533: Use an atomic variables until subinterpreters stop sharing
Python dictionaries. */
_Atomic Py_ssize_t dk_refcnt;
#else
Py_ssize_t dk_refcnt;
#endif

/* Size of the hash table (dk_indices). It must be a power of 2. */
Py_ssize_t dk_size;
Expand Down