Skip to content

docs: clarify wrap_file ownership and lifecycle#3423

Closed
nightcityblade wants to merge 2 commits intopython-trio:mainfrom
nightcityblade:fix/issue-3379
Closed

docs: clarify wrap_file ownership and lifecycle#3423
nightcityblade wants to merge 2 commits intopython-trio:mainfrom
nightcityblade:fix/issue-3379

Conversation

@nightcityblade
Copy link
Copy Markdown
Contributor

Fixes #3379

Document what happens to the wrapped sync file when the async wrapper is closed, whether garbage collection closes it automatically, and why the original sync file should not be used concurrently while wrapped.

Clarify in the wrap_file docstring that closing the async wrapper
closes the underlying file, that GC does not auto-close, and that
the original sync file should not be used concurrently.

Closes python-trio#3379

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@codecov
Copy link
Copy Markdown

codecov bot commented Apr 13, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00000%. Comparing base (3dd35d7) to head (e411fac).
⚠️ Report is 18 commits behind head on main.

Additional details and impacted files
@@               Coverage Diff               @@
##                 main        #3423   +/-   ##
===============================================
  Coverage   100.00000%   100.00000%           
===============================================
  Files             128          128           
  Lines           19424        19436   +12     
  Branches         1318         1318           
===============================================
+ Hits            19424        19436   +12     
Files with missing lines Coverage Δ
src/trio/_file_io.py 100.00000% <ø> (ø)
src/trio/_tests/test_file_io.py 100.00000% <100.00000%> (ø)

... and 9 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Copy Markdown
Contributor

@A5rocks A5rocks left a comment

Choose a reason for hiding this comment

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

  1. please make CI pass
  2. if we don't have tests for this behavior, writing this out in the docs means we promise it... so please add tests!

Comment thread src/trio/_file_io.py Outdated
Comment on lines +506 to +507
file objects. If you need synchronous access, use the
:attr:`~AsyncIOWrapper.wrapped` attribute.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This doesn't make sense, that's literally the file handle in question?

Comment thread src/trio/_file_io.py
Returns:
An :term:`asynchronous file object` that wraps ``file``

The returned wrapper object shares the underlying file with the original
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think maybe this should be indented into the returns section? Not sure about how that would look formatting-wise.

@nightcityblade
Copy link
Copy Markdown
Contributor Author

Thanks, that makes sense. Rather than land wording that over-promises behavior without coverage, I opened a separate docs-only clarification for the related run_process job-control pitfall and I will leave this one for a proper rework with passing CI and tests.

@A5rocks
Copy link
Copy Markdown
Contributor

A5rocks commented Apr 14, 2026

I'm not sure how your comment is related to this PR. What does wrap_file have to do with run_process?

Anyways, it seems you have been opening too many PRs recently... 68 in the last 2 weeks! Unfortunately I think the open issues on Trio need a bunch of thought to how to solve them, so I'm going to close this. Feel free to reopen it whenever you've slowed down how many PRs you make!

@A5rocks A5rocks closed this Apr 14, 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.

trip.wrap_file() undocumented behavior wrt to sync file object

2 participants