Skip to content
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.

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

[C-API Docs] Clarify Constraints on the Arena Allocator #101655

Open
ericsnowcurrently opened this issue Feb 7, 2023 · 0 comments
Open

[C-API Docs] Clarify Constraints on the Arena Allocator #101655

ericsnowcurrently opened this issue Feb 7, 2023 · 0 comments
Labels
3.11 only security fixes 3.12 bugs and security fixes 3.13 bugs and security fixes docs Documentation in the Doc dir topic-C-API

Comments

@ericsnowcurrently
Copy link
Member

The "arena" allocator is used by obmalloc, as well as the code for allocating frames. It is exposed by the following public C-API:

  • PyObject_GetArenaAllocator()
  • PyObject_SetArenaAllocator()

(It is also exposed/used internally as _PyObject_VirtualAlloc() and _PyObject_VirtualFree(). The implementation is exposed internally as _PyMem_ArenaAlloc() and _PyMem_ArenaFree().)

First of all, the docs for PyObject_GetArenaAllocator() are not clear at all about what the function should be used for. I expect it is only meant for use in combination with PyObject_SetArenaAllocator() (e.g. to wrap the old one; to check what's set). That should be clarified.

Second, the C-API docs make it clear that the two public functions may be called before the runtime is initialized. However, the following should be clarified, as there are no documented restrictions AFAICS:

  • may PyObject_SetArenaAllocator() be called after runtime init?
    • status quo: presumably not, since obmalloc would break (similar to the restrictions on PyMem_SetAllocator())
    • internally: we do not ever change the arena allocator
    • recommendation: explicitly document that it must not be called after runtime init (a la PyMem_SetAllocator())
  • may the allocator returned by PyObject_GetArenaAllocator() be used directly?
    • status quo: possibly yes, since people may be doing so at this point (unlikely though)
    • internally: we use it directly (but that's kind of the point)
    • recommendation: explicitly state that it should not be used directly (point to object allocator API instead)
  • ...before runtime init?
    • (relevant only if direct use is okay)
    • status quo: possibly yes, since people may be doing so at this point (unlikely though)
    • internally: we do not use it before runtime init
    • recommendation: at the least document that it shouldn't be used after runtime init
    • better: state that it's only allowed when PyObject_SetArenaAllocator() is allowed (i.e. only before init)
  • must PyObject_GetArenaAllocator() be called with the GIL held after runtime init?
    • (relevant only if post-init PyObject_SetArenaAllocator() is okay)
    • status quo: possibly not, since people haven't needed to before (unlikely though)
    • internally: we always hold the GIL
    • recommendation: recommend (or require) that the GIL be held
  • must the allocator passed to PyObject_SetArenaAllocator() be thread-safe (and reentrant)?
    • status quo: if the allocator may be used directly then possibly yes (since it seems it can be used before runtime init, as well as later without the GIL held); otherwise no (since the PyMem "object" allocator is documented as protected by the GIL)
    • internally: the default arena allocator is thread-safe (and reentrant)
    • recommendation: recommend that the arena allocator be thread-safe (just like the PyMem "raw" allocator)
    • better: explicitly require it (so we can later drop the restriction on the PyMem "mem" and "object" allocators)

If we do specify tighter restrictions than were already documented then we'll need to consider who might be using the public functions in unexpected ways. However, I'd be surprised if there were more than a few (if any) using them at all.


Regardless, ISTM it would be better to deprecate the two public functions and replace them with a new PyConfig field. That would align a lot better with the intention. If there is a need to see if the currently set allocator matches some expected one, then it would be better to have an explicit function for that (e.g. PyObject_IsArenaAllocator()) rather than exposing PyObject_GetArenaAllocator() for that purpose.

The same goes for PyMem_SetAllocator(), where we'd have an additional PyMem_SetAllocatorWrapper() for the after-init case.

@ericsnowcurrently ericsnowcurrently added the docs Documentation in the Doc dir label Feb 7, 2023
@ericsnowcurrently ericsnowcurrently added 3.11 only security fixes topic-C-API 3.12 bugs and security fixes 3.13 bugs and security fixes interpreter-core (Objects, Python, Grammar, and Parser dirs) and removed interpreter-core (Objects, Python, Grammar, and Parser dirs) labels Jul 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3.11 only security fixes 3.12 bugs and security fixes 3.13 bugs and security fixes docs Documentation in the Doc dir topic-C-API
Projects
None yet
Development

No branches or pull requests

1 participant