4 ms·

As far as I know PEP703 includes provisions to make operations on containers thread-safe. That should at least avoid most crashes. Borrowed references can be m

As far as I know PEP703 includes provisions to make operations on containers thread-safe. That should at least avoid most crashes. Borrowed references can be more problematic, but it seems most cases could be fixed by replacing GetItem with FetchItem calls. Overall, as another C Python extension, I don’t really think it’s going to be that much of a pain. In fact, I could even get away with no changes (other than build fixes and such) if, for example, I guarantee that each “main object instance” is only accessed by one thread, and I’d still get a lot of benefits from the nogil.

bjourne 3y ago
Here is code you see all the time in extensions: if (PyList_Size(lst) >= 3) { PyList_SetItem(lst, 0, ...); PyList_SetItem(lst, 1, ...); PyList_SetItem(lst, 2, ...); } Whether the individual operations are thread-safe is irrelevant. For the code to be thread-safe it needs to acquire lst's lock before the if statement and release it afterwards.
- deleted 3y ago
  [deleted]
- deleted 3y ago
  [deleted]
- js2 3y ago
  Your example is not thread safe because the GIL does not protect critical sections like that. The GIL only protects the Python internal interpreter state. PyList_SetItem discards a reference to the item being replaced. If the ref count of the item becomes zero, the item's destructor is run. The destructor can release the GIL. IOW, your example is not much different than the equivalent Python code: if len(lst) >= 3: lst[0] = "..." lst[1] = "..." lst[2] = "..." You'd never expect that to be thread safe either without using something like a threading.RLock() around it. Let's look at the interpreter code: 1. PyList_SetItem calls Py_XSETREF(dst, src) where dst is the list position: https://github.com/python/cpython/blob/ee46cb6aa959d891b0a480fea29f1eb991e0fad8/Objects/listobject.c#L276 https://github.com/python/cpython/blob/ee46cb6aa959d891b0a48... 2. Py_XSETREF calls Py_XDECREF on the old item: https://github.com/python/cpython/blob/ca8b55c7f54b38e264056148075a8061a7082013/Include/cpython/object.h#L376 https://github.com/python/cpython/blob/ca8b55c7f54b38e264056... 3. Py_XDECREF calls Py_DECREF on non-Null objects: https://github.com/python/cpython/blob/main/Include/object.h#L790 https://github.com/python/cpython/blob/main/Include/object.h... 4. Py_DECREF can cause arbitrary code to run: Warning: The deallocation function can cause arbitrary Python code to be invoked (e.g. when a class instance with a __del__() method is deallocated). https://docs.python.org/3/c-api/refcounting.html#c.Py_DECREF https://docs.python.org/3/c-api/refcounting.html#c.Py_DECREF This is backed up by the documentation for PyList_SetItem: This function “steals” a reference to item and discards a reference to an item already in the list at the affected position. https://docs.python.org/3.8/c-api/list.html#c.PyList_SetItem https://docs.python.org/3.8/c-api/list.html#c.PyList_SetItem The "Defining Extension Types: Tutorial" also includes this warning in an example that uses Py_XDECREF: But this would be risky. Our type doesn’t restrict the type of the first member, so it could be any kind of object. It could have a destructor that causes code to be executed that tries to access the first member; or that destructor could release the Global interpreter Lock and let arbitrary code run in other threads that accesses and modifies our object. https://docs.python.org/3/extending/newtypes_tutorial.html#adding-data-and-methods-to-the-basic-example https://docs.python.org/3/extending/newtypes_tutorial.html#a...
  - bjourne 3y ago
    Yes, you are absolutely right, but that doesn't change the fact that many extensions are written as if innocuous function calls like PyList_SetItem won't context switch. It mostly works fine since custom destructors releasing the GIL are extremely rare.
    - js2 3y ago
      "Mostly works fine." I dunno, I'd rather have a reliable bug than a heisenbug. Any class with a __del__() method is going to release the GIL. That's not uncommon. My intuition is that threaded Python is rarer than classes with a __del__() method.

bjourne 3y ago

Here is code you see all the time in extensions: if (PyList_Size(lst) >= 3) { PyList_SetItem(lst, 0, ...); PyList_SetItem(lst, 1, ...); PyList_SetItem(lst, 2, ...); } Whether the individual operations are thread-safe is irrelevant. For the code to be thread-safe it needs to acquire lst's lock before the if statement and release it afterwards.

deleted 3y ago
[deleted]
deleted 3y ago
[deleted]
js2 3y ago
Your example is not thread safe because the GIL does not protect critical sections like that. The GIL only protects the Python internal interpreter state. PyList_SetItem discards a reference to the item being replaced. If the ref count of the item becomes zero, the item's destructor is run. The destructor can release the GIL. IOW, your example is not much different than the equivalent Python code: if len(lst) >= 3: lst[0] = "..." lst[1] = "..." lst[2] = "..." You'd never expect that to be thread safe either without using something like a threading.RLock() around it. Let's look at the interpreter code: 1. PyList_SetItem calls Py_XSETREF(dst, src) where dst is the list position: https://github.com/python/cpython/blob/ee46cb6aa959d891b0a480fea29f1eb991e0fad8/Objects/listobject.c#L276 https://github.com/python/cpython/blob/ee46cb6aa959d891b0a48... 2. Py_XSETREF calls Py_XDECREF on the old item: https://github.com/python/cpython/blob/ca8b55c7f54b38e264056148075a8061a7082013/Include/cpython/object.h#L376 https://github.com/python/cpython/blob/ca8b55c7f54b38e264056... 3. Py_XDECREF calls Py_DECREF on non-Null objects: https://github.com/python/cpython/blob/main/Include/object.h#L790 https://github.com/python/cpython/blob/main/Include/object.h... 4. Py_DECREF can cause arbitrary code to run: Warning: The deallocation function can cause arbitrary Python code to be invoked (e.g. when a class instance with a __del__() method is deallocated). https://docs.python.org/3/c-api/refcounting.html#c.Py_DECREF https://docs.python.org/3/c-api/refcounting.html#c.Py_DECREF This is backed up by the documentation for PyList_SetItem: This function “steals” a reference to item and discards a reference to an item already in the list at the affected position. https://docs.python.org/3.8/c-api/list.html#c.PyList_SetItem https://docs.python.org/3.8/c-api/list.html#c.PyList_SetItem The "Defining Extension Types: Tutorial" also includes this warning in an example that uses Py_XDECREF: But this would be risky. Our type doesn’t restrict the type of the first member, so it could be any kind of object. It could have a destructor that causes code to be executed that tries to access the first member; or that destructor could release the Global interpreter Lock and let arbitrary code run in other threads that accesses and modifies our object. https://docs.python.org/3/extending/newtypes_tutorial.html#adding-data-and-methods-to-the-basic-example https://docs.python.org/3/extending/newtypes_tutorial.html#a...
- bjourne 3y ago
  Yes, you are absolutely right, but that doesn't change the fact that many extensions are written as if innocuous function calls like PyList_SetItem won't context switch. It mostly works fine since custom destructors releasing the GIL are extremely rare.
  - js2 3y ago
    "Mostly works fine." I dunno, I'd rather have a reliable bug than a heisenbug. Any class with a __del__() method is going to release the GIL. That's not uncommon. My intuition is that threaded Python is rarer than classes with a __del__() method.