This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

Author loewis
Recipients kristjan.jonsson, loewis, pitrou
Date 2010-09-13.06:29:27
SpamBayes Score 3.01916e-09
Marked as misclassified No
Message-id <4C8DC4C5.1070009@v.loewis.de>
In-reply-to <1284348269.36.0.854585108754.issue9787@psf.upfronthosting.co.za>
Content
> 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.
History
Date User Action Args
2010-09-13 06:29:30loewissetrecipients: + loewis, pitrou, kristjan.jonsson
2010-09-13 06:29:28loewislinkissue9787 messages
2010-09-13 06:29:27loewiscreate