Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 34k
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
Uh oh!
There was an error while loading. Please reload this page.
Conversation
methane commented Feb 20, 2021 • edited by bedevere-bot
Loading Uh oh!
There was an error while loading. Please reload this page.
edited by bedevere-bot
Uh oh!
There was an error while loading. Please reload this page.
When very large data remains in TextIOWrapper, flush() may fail forever. So prevent large (i.e. 1MiB) data remains in TextIOWrapper internal buffer.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
eryksun commented Feb 20, 2021
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. |
Uh oh!
There was an error while loading. Please reload this page.
methane commented Feb 20, 2021
@eryksun I don't like such tests. Such tests are fragile. |
methane commented Feb 21, 2021
Having buffer in Pure Python TextIOWrapper doesn't make sense. It is just an overhead. |
eryksun commented Feb 21, 2021
That's fine. I just wanted to know if there was more work to do in order to align this behavior with
|
methane commented Feb 21, 2021
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. |
eryksun commented Feb 21, 2021 • edited
Loading Uh oh!
There was an error while loading. Please reload this page.
edited
Uh oh!
There was an error while loading. Please reload this page.
Again, it's nothing to do with performance, but instead matching the observable behavior of |
miss-islington commented Feb 21, 2021
Thanks @methane for the PR 🌮🎉.. I'm working now to backport this PR to: 3.8, 3.9. |
miss-islington commented Feb 21, 2021
Sorry @methane, I had trouble checking out the |
miss-islington commented Feb 21, 2021
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 <songofacandy@gmail.com>
bedevere-bot commented Feb 21, 2021
GH-24617 is a backport of this pull request to the 3.9 branch. |
miss-islington commented Feb 21, 2021
Thanks @methane for the PR 🌮🎉.. I'm working now to backport this PR to: 3.8. |
miss-islington commented Feb 21, 2021
Sorry @methane, I had trouble checking out the |
bedevere-bot commented Feb 21, 2021
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 <songofacandy@gmail.com>
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