Skip to content

gh-151640: Track sharing of BytesIO buffer in free-threaded builds#151651

Merged
ZeroIntensity merged 1 commit into
python:mainfrom
pedramkarimii:gh-151640-bytesio-ft-race
Jul 6, 2026
Merged

gh-151640: Track sharing of BytesIO buffer in free-threaded builds#151651
ZeroIntensity merged 1 commit into
python:mainfrom
pedramkarimii:gh-151640-bytesio-ft-race

Conversation

@pedramkarimii

@pedramkarimii pedramkarimii commented Jun 18, 2026

Copy link
Copy Markdown
Contributor

Fixes gh-151640.

This replaces the earlier always-copy approach with the shared-buffer tracking / copy-on-write design proposed in review.

In free-threaded builds, whole-buffer read paths, peek(), and getvalue() can expose the internal bytes object. BytesIO marks that buffer as shared, and a later write detaches to a private buffer before mutating or resizing it. This preserves the zero-copy fast path when the buffer has not been exposed.

The change includes:

  • a concurrent regression test covering whole-buffer reads, peeks, and getvalue() with buffer-resizing writes;
  • the corresponding BytesIO.__sizeof__ expectation adjustment; and
  • a NEWS entry.

Local validation:

  • regular-GIL and free-threaded builds;
  • targeted test_io and test_free_threading.test_io;
  • 20 repeated concurrent-race runs;
  • free-threaded Py_DEBUG and -R 3:3;
  • free-threaded Py_DEBUG ThreadSanitizer: targeted regression passed 10/10 with no diagnostics; and
  • make patchcheck.

@bedevere-app

bedevere-app Bot commented Jun 18, 2026

Copy link
Copy Markdown

Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool.

If this change has little impact on Python users, wait for a maintainer to apply the skip news label instead.

@python-cla-bot

python-cla-bot Bot commented Jun 18, 2026

Copy link
Copy Markdown

All commit authors signed the Contributor License Agreement.

CLA signed

@bedevere-app

bedevere-app Bot commented Jun 18, 2026

Copy link
Copy Markdown

Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool.

If this change has little impact on Python users, wait for a maintainer to apply the skip news label instead.

@pedramkarimii

Copy link
Copy Markdown
Contributor Author

Additional context:

This PR is intended to fix the specific _io.BytesIO free-threaded race reported in gh-151640. The issue report focuses on the whole-buffer BytesIO.read() fast path returning self->buf directly via Py_NewRef(), while a concurrent writer may later resize that same internal bytes object.

While investigating the code path, I also found that BytesIO.getvalue() can expose the same internal buffer in a similar way. For that reason, this PR handles both whole-buffer read() and getvalue() in Py_GIL_DISABLED builds.

The chosen fix is intentionally conservative: in free-threaded builds, avoid returning the internal BytesIO buffer directly to user code, while keeping the existing fast path unchanged for regular GIL builds. This avoids changing behavior/performance for non-free-threaded builds and keeps the patch localized to _io.BytesIO.

I also considered fixing this lower in the resize/shared-buffer ownership path, but that seemed more invasive because it would require changing the interaction between SHARED_BUF(), _PyBytes_Resize(), and buffer ownership/copy-on-write behavior. If maintainers prefer preserving this optimization in free-threaded builds, I’m happy to rework the patch in that direction.

Comment thread Modules/_io/bytesio.c
@pedramkarimii pedramkarimii force-pushed the gh-151640-bytesio-ft-race branch 4 times, most recently from b26fbb8 to 33bc6c8 Compare June 27, 2026 11:06
@nascheme

nascheme commented Jul 3, 2026

Copy link
Copy Markdown
Member

I think always copying buf is going to cause too much performance regression. In some cases the buffer is going to be large and so this copy will be significant. Tracking if the buf has been shared seems like a better solution. That sounds like your "more invasive" approach, which I think is worth the extra work and complexity. I prototyped this change here: https://github.com/nascheme/cpython/tree/gh-151640-bytesio-shared-flag

Keep zero-copy whole-buffer reads, but detach to a private buffer before a later write can mutate or resize an exposed BytesIO buffer.
@pedramkarimii pedramkarimii force-pushed the gh-151640-bytesio-ft-race branch from 33bc6c8 to 367b372 Compare July 4, 2026 05:18
@pedramkarimii pedramkarimii changed the title gh-151640: avoid sharing BytesIO buffer in free-threaded builds gh-151640: Track sharing of BytesIO buffer in free-threaded builds Jul 4, 2026
@pedramkarimii

Copy link
Copy Markdown
Contributor Author

@nascheme Thanks for the prototype. I reworked the PR to follow the shared-buffer flag / copy-on-write approach, rather than always copying the buffer in free-threaded builds.

The update includes the concurrent regression test and the NEWS entry. All CI checks are now green. Local validation also included free-threaded Py_DEBUG/refleak and the targeted free-threaded ThreadSanitizer regression: 10/10 passes with no diagnostics.

The updated commit is 367b372.

@nascheme

nascheme commented Jul 4, 2026

Copy link
Copy Markdown
Member

I'll let @ZeroIntensity take a look too but I think this can be merged.

@ZeroIntensity ZeroIntensity 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

@ZeroIntensity ZeroIntensity added needs backport to 3.14 bugs and security fixes needs backport to 3.15 pre-release feature fixes, bugs and security fixes labels Jul 6, 2026
@ZeroIntensity ZeroIntensity merged commit 881a13a into python:main Jul 6, 2026
62 checks passed
@miss-islington-app

Copy link
Copy Markdown

Thanks @pedramkarimii for the PR, and @ZeroIntensity for merging it 🌮🎉.. I'm working now to backport this PR to: 3.14, 3.15.
🐍🍒⛏🤖

@miss-islington-app

Copy link
Copy Markdown

Sorry, @pedramkarimii and @ZeroIntensity, I could not cleanly backport this to 3.15 due to a conflict.
Please backport using cherry_picker on command line.

cherry_picker 881a13a97c7a29b9de57913c773cd5f195270fbd 3.15

@miss-islington-app

Copy link
Copy Markdown

Sorry, @pedramkarimii and @ZeroIntensity, I could not cleanly backport this to 3.14 due to a conflict.
Please backport using cherry_picker on command line.

cherry_picker 881a13a97c7a29b9de57913c773cd5f195270fbd 3.14

@bedevere-app

bedevere-app Bot commented Jul 6, 2026

Copy link
Copy Markdown

GH-153151 is a backport of this pull request to the 3.15 branch.

@bedevere-app bedevere-app Bot removed the needs backport to 3.15 pre-release feature fixes, bugs and security fixes label Jul 6, 2026
@bedevere-app

bedevere-app Bot commented Jul 6, 2026

Copy link
Copy Markdown

GH-153153 is a backport of this pull request to the 3.14 branch.

@bedevere-app bedevere-app Bot removed the needs backport to 3.14 bugs and security fixes label Jul 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants