Skip to content

gh-135893: Lazily initialize the libffi allocator in ctypes - #161844

Open
ZeroIntensity wants to merge 4 commits into
python:mainfrom
ZeroIntensity:ctypes/lazily-load-libffi-closure
Open

ZeroIntensity wants to merge 4 commits into
python:mainfrom
ZeroIntensity:ctypes/lazily-load-libffi-closure

Conversation

@ZeroIntensity

@ZeroIntensity ZeroIntensity commented Nov 12, 2026 •

Copy link
Copy Markdown
Member

This prevents libffi from causing side effects when ctypes is imported.

@ZeroIntensity ZeroIntensity added the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Nov 12, 2026
@bedevere-bot

Copy link
Copy Markdown

🤖 New build scheduled with the buildbot fleet by @ZeroIntensity for commit 878a8a5 🤖

Results will be shown at:

https://buildbot.python.org/all/#/grid?branch=refs%2Fpull%2F161844%2Fmerge

If you want to schedule another build, you need to add the 🔨 test-with-buildbots label again.

@bedevere-bot bedevere-bot removed the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Nov 12, 2026
@mhsmith

mhsmith commented Nov 12, 2026

Copy link
Copy Markdown
Member

The Android failure was caused by the buildbot running out of disk space, and is probably not relevant to this PR.

@vstinner vstinner left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you run #128485 reproducer using TSAN to check that your change doesn't reintroduce the bug?

cc @kumaraditya303

// See https://github.com/python/cpython/issues/128485
// This allocates some memory and then frees it to ensure that libffi's
// dlmalloc allocator initializes itself exactly once, avoiding data races
// between threads that don't share a GIL.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand well how _PyOnceFlag_CallOnce() avoid reintroducing #128485. If two threads call _ctypes_alloc_callback(), the first one calls this closure_alloc_init() function and the other one calls Py_ffi_closure_alloc() which is a wrapper to ffi_closure_alloc(). At the end, the two threads call ``ffi_closure_alloc()` in parallel and we are back to #128485 issue, no?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The whole point of a once flag is that multiple threads can't call it in parallel. It's called once, per the name. When _PyOnceFlag_CallOnce is called concurrently, the other threads will wait until the winner finishes initialization. Here's the scenario:

  1. Thread 1 enters _ctypes_alloc_callback.
  2. Thread 2 enters the same code path.
  3. Thread 1 calls _PyOnceFlag_CallOnce.
  4. The once flag is unlocked, so the thread calls closure_alloc_init to initialize the libffi allocator.
  5. Thread 2 calls _PyOnceFlag_CallOnce at the same time, but it sees that it's in the "locked" state, so it spins until thread 1 marks the once flag as initialized.
  6. Thread 2 does not call _ctypes_alloc_callback and proceeds; the allocator has been initialized by thread 1.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thread 2 calls _PyOnceFlag_CallOnce at the same time, but it sees that it's in the "locked" state, so it spins until thread 1 marks the once flag as initialized.

Oh, I missed that part of _PyOnceFlag_CallOnce(). I didn't know that other threads block until the function is initialized. In that case, ok, I understand that the change doesn't reintroduce the bug.

@ZeroIntensity

Copy link
Copy Markdown
Member Author

Did you run #128485 reproducer using TSAN to check that your change doesn't reintroduce the bug?

No. I had assumed that the fix had come with a test case, but apparently not. I think it would make sense to add a test for this rather than manually verifying.

@vstinner vstinner left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

awaiting merge needs backport to 3.14 bugs and security fixes needs backport to 3.15 bugs and security fixes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants