[issue9787] Release the TLS lock during allocations

[Martin v. Löwis]

Martin v. Löwis <martin at v.loewis.de> added the comment:

> You may find this hard to believe, but we do in fact embed python
> into other applications.

This is actually very easy to believe.

> appMalloc, is in this case, the canonical memory allocator in
> UnrealEngine.  But it could be any other memory allocator so that is
> beside the point.

This seems to be the core of the issue. Any other memory allocator
would *not* inquire about the state of Python. Any other memory
allocator would not even be aware that it is used by Python.

> What appMalloc is doing, in this case, is for every allocation, to
> get the python TLS pointer.  There is nothing wrong with this

I find this wrong. It violates the software layering. The memory
management layer is not supposed to access upper layers (such as
the interpreter state, or the application state).

> Now, regardless of the above, surely it is an improvement in general
> if we make tighter use of the TLS lock.  It's not a good idea to hold
> this lock across malloc calls if we can avoid it.  The patch is
> harmless, might even be an improvement, so why object to it?

The code change itself is harmless, yes. The comment is not. It imposes
a requirement on Python (namely, that the malloc implementation may
be free to make calls to Python) which is harmful. The malloc
implementation just has no business looking at the thread state.

So I remain -1 on this change.

----------

_______________________________________
Python tracker <report at bugs.python.org>
<http://bugs.python.org/issue9787>
_______________________________________