Handle reentrant detach in TextIOWrapper.close() by ever0de · Pull Request #7277 · RustPython/RustPython

ever0de · 2026-02-28T16:36:21Z

When a custom closed property on the underlying buffer calls detach() during the file_closed() check in close(), the wrapper's internal buffer becomes None. Subsequent flush/close operations then fail with AttributeError on NoneType.

Add a guard after the file_closed() check to detect if the buffer was detached reentrantly, and return early in that case (detach has already flushed the stream).

This mirrors the fix applied in CPython
(https://github.com/python/cpython/issues/142594\).

Summary by CodeRabbit

Bug Fixes

Fixed a potential edge case during file closing operations that could cause issues when buffer detachment occurs reentrantly, improving the stability of file I/O operations.

When a custom `closed` property on the underlying buffer calls `detach()` during the `file_closed()` check in `close()`, the wrapper's internal buffer becomes None. Subsequent flush/close operations then fail with AttributeError on NoneType. Add a guard after the `file_closed()` check to detect if the buffer was detached reentrantly, and return early in that case (detach has already flushed the stream). This mirrors the fix applied in CPython (https://github.com/python/cpython/issues/142594\).

coderabbitai · 2026-02-28T16:36:43Z

📝 Walkthrough

Walkthrough

Adds early-return guards in the TextIOWrapper close sequence to prevent operations on a detached buffer when reentrant calls to detach() occur during the file closure process, mitigating CPython-like reentrancy issues.

Changes

Cohort / File(s)	Summary
I/O Buffer Reentrancy Guard `crates/vm/src/stdlib/io.rs`	Introduces early-return checks in the close path that detect if the buffer has been detached (set to None) and exit gracefully, preventing further operations on a null buffer when reentrant detach calls occur.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

RustPython/RustPython#6267: Directly related—adds the detach implementation and flush helper that sets the buffer to None, which this PR guards against during close operations.

Suggested reviewers

youknowone

Poem

🐰 A buffer once detached mid-close,
Would cause reentrancy's throes,
But now with guards so neat and tight,
We check, return, and all's right! ✨

🚥 Pre-merge checks | ✅ 3

✅ 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 clearly summarizes the main change: adding a reentrant detach guard to TextIOWrapper.close(), which directly matches the code changes and PR objectives.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

_{✏️ 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.}

github-actions · 2026-02-28T16:37:35Z

📦 Library Dependencies

The following Lib/ modules were modified. Here are their dependencies:

Error: 'frozenset' object has no attribute 'discard'

Legend:

[+] path exists in CPython
[x] up-to-date, [ ] outdated

coderabbitai

🧹 Nitpick comments (1)

crates/vm/src/stdlib/io.rs (1)

3796-3802: Please add a regression test for this reentrant path.

A targeted test where the underlying buffer’s closed accessor reentrantly calls TextIOWrapper.detach() during close() would lock this fix in and prevent regressions.

🤖 Prompt for AI Agents

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

In `@crates/vm/src/stdlib/io.rs` around lines 3796 - 3802, Add a regression test
that exercises the reentrant detach path by creating a TextIOWrapper-like object
whose closed property accessor calls TextIOWrapper.detach() during close(); the
test should call the wrapper's close() and assert no panic and that buffer ends
up detached (matching the vm.is_none(&zelf.lock(vm)?.buffer) early-return
behavior). Locate the test near existing io tests for TextIOWrapper/close and
reference the methods file_closed(), detach(), and the closed property to
simulate the reentrant call and verify the early-return branch is hit.

🤖 Prompt for all review comments with AI agents

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

Nitpick comments:
In `@crates/vm/src/stdlib/io.rs`:
- Around line 3796-3802: Add a regression test that exercises the reentrant
detach path by creating a TextIOWrapper-like object whose closed property
accessor calls TextIOWrapper.detach() during close(); the test should call the
wrapper's close() and assert no panic and that buffer ends up detached (matching
the vm.is_none(&zelf.lock(vm)?.buffer) early-return behavior). Locate the test
near existing io tests for TextIOWrapper/close and reference the methods
file_closed(), detach(), and the closed property to simulate the reentrant call
and verify the early-return branch is hit.

ℹ️ 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 f6d58c8.

⛔ Files ignored due to path filters (1)

Lib/test/test_io.py is excluded by !Lib/**

📒 Files selected for processing (1)

crates/vm/src/stdlib/io.rs

ever0de · 2026-02-28T16:54:53Z

@coderabbitai I have enabled test_issue142594 in this PR which does exactly what you suggested (calling wrapper.detach() inside the closed property). It should prevent regressions for this fix going forward.

coderabbitai · 2026-02-28T16:56:11Z

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Search for test_issue142594 in the codebase
rg -n "test_issue142594" -A 30

Length of output: 0

🏁 Script executed:

#!/bin/bash
# Search for 142594 in the codebase
rg -n "142594" -C 5

# Also search for test files related to io
fd -e py test.*io

Length of output: 933

🏁 Script executed:

#!/bin/bash
# Search more broadly for test files and the test
fd -t f "test_" | head -20

# Look for any references to the test being skipped or enabled
rg -n "issue142594" -i

# Check common test directories
fd -t d -d 2 "test"

Length of output: 529

🏁 Script executed:

#!/bin/bash
# Search for test files in Lib directory
fd -t f "test_io" Lib/

# Search for the test name in Lib
rg -n "test_issue142594" Lib/ -A 20

# Also check for any recent changes to test files
git diff --name-only origin/main | grep -i test

Length of output: 156

🏁 Script executed:

#!/bin/bash
# Look at the git diff for test_io.py to see what changed
git diff origin/main -- Lib/test/test_io.py | head -100

# Also search the current version of the file
rg -n "test_issue142594" --type py -A 25

Length of output: 491

@ever0de, excellent! I can see you've removed the @unittest.expectedFailure decorator from test_issue142594 in Lib/test/test_io.py. The test was previously marked as expected failure with the comment:

# TODO: RUSTPYTHON; AttributeError: 'NoneType' object has no attribute 'closed'

This is exactly the issue your fix addresses! By enabling this test, you've ensured that:

The reentrant detach() scenario (called via a custom closed property) is properly handled
The fix will be protected against future regressions
RustPython's behavior now matches CPython for this edge case

This is a great way to validate the fix and maintain test coverage for this subtle reentrancy issue. The test suite will now catch any future changes that might reintroduce the AttributeError on NoneType problem. 🎉

    (\(\
    ( -.-)
    o_(")(")
    [Test coverage achieved!]

When a custom `closed` property on the underlying buffer calls `detach()` during the `file_closed()` check in `close()`, the wrapper's internal buffer becomes None. Subsequent flush/close operations then fail with AttributeError on NoneType. Add a guard after the `file_closed()` check to detect if the buffer was detached reentrantly, and return early in that case (detach has already flushed the stream). This mirrors the fix applied in CPython (https://github.com/python/cpython/issues/142594\).

coderabbitai Bot reviewed Feb 28, 2026

View reviewed changes

youknowone approved these changes Mar 1, 2026

View reviewed changes

youknowone merged commit be39a67 into RustPython:main Mar 1, 2026
14 checks passed

Navigation Menu

Repository navigation

Handle reentrant detach in TextIOWrapper.close() - #7277

Handle reentrant detach in TextIOWrapper.close()#7277
youknowone merged 1 commit into
RustPython:mainfrom
ever0de:fix/textiowrapper-close-reentrant-detach

ever0de commented Feb 28, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Feb 28, 2026 •

edited

Loading

Walkthrough

Changes

Estimated code review effort

Possibly related PRs

Suggested reviewers

Poem

Uh oh!

github-actions Bot commented Feb 28, 2026

Uh oh!

coderabbitai Bot left a comment

Uh oh!

ever0de commented Feb 28, 2026

Uh oh!

coderabbitai Bot commented Feb 28, 2026

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

ever0de commented Feb 28, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Bug Fixes

Uh oh!

coderabbitai Bot commented Feb 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Possibly related PRs

Suggested reviewers

Poem

Uh oh!

github-actions Bot commented Feb 28, 2026

📦 Library Dependencies

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

ever0de commented Feb 28, 2026

Uh oh!

coderabbitai Bot commented Feb 28, 2026

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

ever0de commented Feb 28, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Feb 28, 2026 •

edited

Loading