[issue8020] Crash in Py_ADDRESS_IN_RANGE macro

Antoine Pitrou added the comment: I've committed Matt's patch in r87835 (3.2), r87836 (3.1) and r87837 (2.7). Thank you for contributing! -- resolution: -> fixed stage: needs patch -> committed/rejected status: open -> closed versions: -Python 2.6 ___

[issue8020] Crash in Py_ADDRESS_IN_RANGE macro

Tim Peters added the comment: Cool! Python has a long history of catering to feeble compilers too ;-), so using function locals is fine. Speed matters more than obscurity here. -- ___ Python tracker

[issue8020] Crash in Py_ADDRESS_IN_RANGE macro

Matt Bandy added the comment: Sorry, I should have been more precise. I did use static for the variable I declared at file scope. Looking at the disassembly again also reminded me of something I should have mentioned in the original bug report: at least when using Visual C++ 2008, in the re

[issue8020] Crash in Py_ADDRESS_IN_RANGE macro

Tim Peters added the comment: Whatever works best. Unsure what you mean by "global", though, because it's not a C concept. File-scope variables in C can have internal or external linkage; to get internal linkage, so that lifetime analysis is effective without needing (roughly speaking) cros

[issue8020] Crash in Py_ADDRESS_IN_RANGE macro

Matt Bandy added the comment: I tried this out in VC++ 2008 with the new temporary as both a global variable and a local. At least for VC++, Amaury is correct -- the compiler is generating the store to the global even though it is never read, so making it a local instead does save an instruc

[issue8020] Crash in Py_ADDRESS_IN_RANGE macro

Tim Peters added the comment: > `x` should be a local variable. ... assembler code will be smaller. ??? It should make no difference to an optimizing compiler. Lifetime analysis (even if it's feeble) should be able to determine that all uses are purely local, with no need to store the value a

[issue8020] Crash in Py_ADDRESS_IN_RANGE macro

Matt Bandy added the comment: That should probably be: #define Py_ADDRESS_IN_RANGE(P, POOL)\ ((x = (POOL)->arenaindex) < maxarenas &&\ (uptr)(P) - arenas[x].address < (uptr)ARENA_SIZE && \ arenas[x].address != 0) The address in the

[issue8020] Crash in Py_ADDRESS_IN_RANGE macro

Tim Peters added the comment: Right, I already agreed it would be fine to fix this if it's cheap ;-) I didn't give any real thought to the macro solution, but that's fine by me too. It would be best to embed the assignment in a _natural_ place, though; like, say: #define Py_ADDRESS_IN_RANGE

[issue8020] Crash in Py_ADDRESS_IN_RANGE macro

Matt Bandy added the comment: It's a pretty major limitation on the embedding case if you can't allow other threads that aren't related to Python to run at any time that another thread may be in obmalloc, and one I haven't seen documented anywhere. The only other fix that occurs to me would

[issue8020] Crash in Py_ADDRESS_IN_RANGE macro

Tim Peters added the comment: Just noting there are ways to trick the macro into reading a thing once. For example, embed something like a ((x = arenas[(POOL)->arenaindex].address) || 1) clause early on & refer to x later (where x is some file-scope new variable). obmalloc is not intended t

[issue8020] Crash in Py_ADDRESS_IN_RANGE macro

Tim Peters added the comment: Amaury, I have no real idea what your "No, the description above says it: ..." is a response to, but if it's to my point that obmalloc is not intended to be thread-safe, it's not addressing that point. Any case in which another thread can affect a thread current

[issue8020] Crash in Py_ADDRESS_IN_RANGE macro

Matt Bandy added the comment: Amaury is correct -- in the case I observed, the other thread had nothing to do with Python and was not calling Python functions that would have required obtaining the GIL at any time (again, I'm using an embedded Python interpreter in a larger app, not the stand

[issue8020] Crash in Py_ADDRESS_IN_RANGE macro

Amaury Forgeot d'Arc added the comment: No, the description above says it: only the first thread calls Py_ADDRESS_IN_RANGE. The other thread happens to modify the memory at the beginning of the page, just at the address (POOL)->arenaindex. Could we use an "inline" function instead of a macro?

[issue8020] Crash in Py_ADDRESS_IN_RANGE macro

Tim Peters added the comment: Note that nothing in obmalloc is _intended_ to be thread-safe. Almost all Python API functions make the caller responsible for serializing calls (which is usually accomplished "by magic" via the GIL). This includes calls to all facilities supplied by obmalloc.

[issue8020] Crash in Py_ADDRESS_IN_RANGE macro

Antoine Pitrou added the comment: Obviously it also involves changing the code where the macro is invoked. It would be quite non-controversial though. I've a got a more interesting question: how does the memory allocator end up being invoked from two threads at once? Usually memory allocation

[issue8020] Crash in Py_ADDRESS_IN_RANGE macro

Matt Bandy added the comment: You can't add a block without changing the way the macro is used -- it's usually used like: if (Py_ADDRESS_IN_RANGE(p, pool)) This won't work with your proposed change since you can't put a do {} loop into the test expression of the if statement. -- __

[issue8020] Crash in Py_ADDRESS_IN_RANGE macro

New submission from Matt Bandy : Using the Py_ADDRESS_IN_RANGE macro can result in a crash under certain threading conditions. Since it is intentionally possible for (POOL)->arenaindex to reference memory which is not under Python's control, another thread may modify that memory. In particul