-
-
Notifications
You must be signed in to change notification settings - Fork 32.3k
bpo-43260: io: Prevent large data remains in textio buffer. #24592
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
When very large data remains in TextIOWrapper, flush() may fail forever. So prevent large (i.e. 1MiB) data remains in TextIOWrapper internal buffer.
This needs a test in a subprocess. In POSIX the maximum available address space or heap size can be easily controlled. For example:
It's possible in Windows with kernel Job objects, but Python's standard library doesn't support them, not without ctypes. |
@eryksun I don't like such tests. Such tests are fragile. |
Having buffer in Pure Python TextIOWrapper doesn't make sense. It is just an overhead. |
That's fine. I just wanted to know if there was more work to do in order to align this behavior with
|
I don't think so. There is a BufferedIO under the TextIOWrapper. No need to add one additonal buffer layer. C implementation of TextIOWrapper is fast. Since it is very fast, calling BufferedIO.write() is slower than appending internal buffer. On the other hand, Python implementation is not so fast. Calling overhead is not so large. So adding one more buffering layer isn't worth enough. |
Again, it's nothing to do with performance, but instead matching the observable behavior of |
Thanks @methane for the PR 🌮🎉.. I'm working now to backport this PR to: 3.8, 3.9. |
Sorry @methane, I had trouble checking out the |
Sorry, @methane, I could not cleanly backport this to |
…thonGH-24592) When very large data remains in TextIOWrapper, flush() may fail forever. So prevent that data larger than chunk_size is remained in TextIOWrapper internal buffer. Co-Authored-By: Eryk Sun (cherry picked from commit 01806d5) Co-authored-by: Inada Naoki <[email protected]>
GH-24617 is a backport of this pull request to the 3.9 branch. |
Thanks @methane for the PR 🌮🎉.. I'm working now to backport this PR to: 3.8. |
Sorry @methane, I had trouble checking out the |
GH-24618 is a backport of this pull request to the 3.8 branch. |
…thonGH-24592) When very large data remains in TextIOWrapper, flush() may fail forever. So prevent that data larger than chunk_size is remained in TextIOWrapper internal buffer. Co-Authored-By: Eryk Sun. (cherry picked from commit 01806d5) Co-authored-by: Inada Naoki <[email protected]>
When very large data remains in TextIOWrapper, flush() may fail forever. So prevent that data larger than chunk_size is remained in TextIOWrapper internal buffer. Co-Authored-By: Eryk Sun (cherry picked from commit 01806d5)
When very large data remains in TextIOWrapper, flush() may fail forever. So prevent that data larger than chunk_size is remained in TextIOWrapper internal buffer. Co-Authored-By: Eryk Sun. (cherry picked from commit 01806d5)
…-24592) When very large data remains in TextIOWrapper, flush() may fail forever. So prevent that data larger than chunk_size is remained in TextIOWrapper internal buffer. Co-Authored-By: Eryk Sun
When very large data remains in TextIOWrapper, it can not flush the internal
buffer forever because of MemoryError.
bpo-43260