`io.TextIoWrapper.reconfigure` to always flush#7281

ShaharNaveh

Summary by CodeRabbit

Bug Fixes
- Improved I/O buffer flushing behavior during reconfiguration to unconditionally flush all pending data, ensuring more consistent I/O operations when encoding or other parameters change.
Tests
- Added test cases validating error handling and behavior during encoding reconfiguration scenarios.

coderabbitai

📝 Walkthrough

Walkthrough

The PR fixes a panic in TextIOWrapper's reentrant reconfigure call by unconditionally flushing pending data after handling encoding/error/newline changes, rather than conditionally based on a flag. A test case reproducing the reentrant scenario is added.

Changes

Cohort / File(s)	Summary
Reconfigure Flush Logic `crates/vm/src/stdlib/io.rs`	Removed `flush_on_reconfigure` flag and reworked reconfigure flow to unconditionally flush any pending data and underlying buffer after handling configuration changes, eliminating conditional flush behavior.
Reentrancy Test Coverage `extra_tests/snippets/stdlib_io.py`	Added import of `TextIOWrapper` from `io` module and created `Gh6588` test class that reproduces the reentrant `writelines` scenario by reconfiguring TextIOWrapper encoding during a write operation.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Possibly related PRs

#6565 — Modifies the same io.rs flush and reconfigure behavior with buffering/flush adjustments (flush-before-read and related flush paths).

Poem

🐰 Flush it all away, no flags to delay,
Reconfigure flows on a clearer path today,
Reentrancy's panic now stays at bay,
One unconditional flush saves the day! 💧

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name	Status	Explanation	Resolution
Linked Issues check	⚠️ Warning	The PR fails to address the core requirements: preventing Rust panics and detecting/rejecting reentrant calls to TextIOWrapper.reconfigure.	The changes only unconditionally flush data but do not implement mutex locking improvements or reentrant call detection to prevent panics as required by issue `#6588`.
Docstring Coverage	⚠️ Warning	Docstring coverage is 16.67% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately describes the main change: making TextIOWrapper.reconfigure always flush, which is directly reflected in the code changes.
Out of Scope Changes check	✅ Passed	The code changes are minimal and directly related to the PR objective of ensuring TextIOWrapper.reconfigure always flushes, with no extraneous modifications.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests
Post copyable unit tests in a comment

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

coderabbitai

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@extra_tests/snippets/stdlib_io.py`:
- Line 2: Remove the unused imports BytesIO and StringIO from the import
statement in stdlib_io.py; edit the line that currently imports BufferedReader,
BytesIO, FileIO, RawIOBase, StringIO, TextIOWrapper and delete BytesIO and
StringIO so only actually used symbols (e.g., BufferedReader, FileIO, RawIOBase,
TextIOWrapper) remain, resolving the F401 lint warnings.
- Around line 63-86: The test fails early with AttributeError because Gh6588
lacks a flush method, so TextIOWrapper.writelines() raises instead of exercising
the reentrant reconfigure-path; update the Gh6588 class to implement a
flush(self) method (e.g., forward to self.textio.flush() or a no-op) so
TextIOWrapper (created as textio via TextIOWrapper(raw, ...)) can call flush and
the test will exercise the reentrant call in write/reconfigure rather than
short-circuiting on missing flush.

ℹ️ Review info

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 2833cd8 and 53fca08.

📒 Files selected for processing (2)

crates/vm/src/stdlib/io.rs
extra_tests/snippets/stdlib_io.py

* Always flush on reconfigure (io.TextWrapper) * Add test

ShaharNaveh added 2 commits February 28, 2026 17:49

Always flush on reconfigure (io.TextWrapper)

954630a

Add test

53fca08

ShaharNaveh marked this pull request as ready for review March 1, 2026 00:48

coderabbitai Bot reviewed Mar 1, 2026

View reviewed changes

youknowone approved these changes Mar 1, 2026

View reviewed changes

youknowone pushed a commit to youknowone/RustPython that referenced this pull request Mar 22, 2026

io.TextIoWrapper.reconfigure to always flush (RustPython#7281) …

f0ab49a

* Always flush on reconfigure (io.TextWrapper) * Add test

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

`io.TextIoWrapper.reconfigure` to always flush#7281