bpo-24658: Fix read/write on file with a size greater than 2GB on OSX #1705
Conversation
@matrixise, thanks for your PR! By analyzing the history of the files in this pull request, we identified @serhiy-storchaka, @zooba and @benjaminp to be potential reviewers. |
Needs a couple of minor changes, but otherwise looks pretty good. I'll test it on my machine once Travis and AppVeyor are happy with it. |
self.assertEqual(f.tell(), size) | ||
|
||
with self.open(TESTFN, "rb") as f: | ||
self.assertEqual(f.read(size), size) |
with self.open(TESTFN, "wb") as f: | ||
b = b'x' * size | ||
self.assertEqual(f.write(b), size) | ||
self.assertEqual(f.tell(), size) |
zware
May 22, 2017
Member
Since this test is writing the full file, it will need to remove it as well so that the other tests will be set up as expected.
@@ -19,6 +19,17 @@ PyAPI_FUNC(char*) Py_EncodeLocale( | |||
|
|||
PyAPI_FUNC(PyObject *) _Py_device_encoding(int); | |||
|
|||
#if defined(MS_WINDOWS) || defined(__APPLE__) | |||
/* On Windows, the count parameter of read() is an int | |||
See issue #24658 |
zware
May 22, 2017
Member
This makes it sound like Windows is dealt with by bpo-24658, but it's actually macOS in that issue. There should be some brief text added about macOS. If you can dig up an issue number about Windows, that could be added as well.
with self.open(TESTFN, "wb") as f: | ||
b = b'x' * size | ||
self.assertEqual(f.write(b), size) | ||
self.assertEqual(f.tell(), size) |
@@ -45,6 +45,20 @@ def tearDownClass(cls): | |||
raise cls.failureException('File was not truncated by opening ' | |||
'with mode "wb"') | |||
|
|||
@bigmemtest(size=_2G + 1024 * 1024, memuse=2) |
vstinner
May 22, 2017
Member
Are you sure about the memuse=2 parameter? I would only expect memuse=1. Try maybe tracemalloc to get the memory peak: run tests using python3 -X tracemalloc ... and then display tracemalloc.get_traced_memory()[1].
https://docs.python.org/dev/library/tracemalloc.html#tracemalloc.get_traced_memory
(You can run such test on Linux.)
if (size > INT_MAX) | ||
size = INT_MAX; | ||
#endif | ||
if (size > _PY_READ_MAX) { |
vstinner
May 22, 2017
Member
I'm not sure that it's ok to use this if on OS other than macOS and Windows, since size type is ssize_t. I expect "condition is always false" warnings on many recent compilers. I would prefer to keep an #ifdef.
|
@@ -21,7 +21,7 @@ PyAPI_FUNC(PyObject *) _Py_device_encoding(int); | |||
|
|||
#if defined(MS_WINDOWS) || defined(__APPLE__) | |||
/* On Windows, the count parameter of read() is an int | |||
See issue #24658 | |||
Add the support of MacOS with the issue #24658 |
@@ -81,7 +81,7 @@ before_script: | |||
script: | |||
# `-r -w` implicitly provided through `make buildbottest`. | |||
- make buildbottest TESTOPTS="-j4" | |||
- make buildbottest TESTOPTS="-j4 -v test_largefile" |
matrixise
May 23, 2017
Author
Member
yep +1, just for some tests, but I will rebase my branch before the final PR.
@@ -5,7 +5,7 @@ | |||
import stat | |||
import sys | |||
import unittest | |||
from test.support import TESTFN, requires, unlink | |||
from test.support import TESTFN, requires, unlink, bigmemtest, _2G |
@@ -45,6 +45,25 @@ def tearDownClass(cls): | |||
raise cls.failureException('File was not truncated by opening ' | |||
'with mode "wb"') | |||
|
|||
def test_large_reads_writes(self): | |||
# see issue #24658 | |||
requires('largefile', |
@matrixise Looks like the current Travis failure is due to |
I am not aware of segfault, only of process *killed* on Travis which
probably means that the test uses too much resources (memory?).
|
Ah, ok. |
To try and help move older pull requests forward, we are going through and backfilling 'awaiting' labels on pull requests that are lacking the label. Based on the current reviews, the best we can tell in an automated fashion is that a core developer requested changes to be made to this pull request. If/when the requested changes have been made, please leave a comment that says, |
bpo-24658: Fix read/write on file with a size greater than 2GB on OSX
…X and _PY_WRITE_MAX
Thanks @matrixise for the PR, and @vstinner for merging it |
Thanks @matrixise for the PR, and @vstinner for merging it |
Thanks @matrixise for the PR, and @vstinner for merging it |
On macOS, fix reading from and writing into a file with a size larger than 2 GiB. (cherry picked from commit 74a8b6e) Co-authored-by: Stéphane Wirtel <stephane@wirtel.be>
GH-9936 is a backport of this pull request to the 3.7 branch. |
Sorry, @matrixise and @vstinner, I could not cleanly backport this to |
Sorry, @matrixise and @vstinner, I could not cleanly backport this to |
@matrixise: @miss-islington did the 3.7 backport, but failed to create 2.7 and 3.6 backports. Can you please try to backport your change to these branches? |
GH-9937 is a backport of this pull request to the 3.6 branch. |
…-1705) On macOS, fix reading from and writing into a file with a size larger than 2 GiB.
GH-9938 is a backport of this pull request to the 2.7 branch. |
…-1705) On macOS, fix reading from and writing into a file with a size larger than 2 GiB.
Good work! I like to observe how you collaborated, @matrixise and @vstinner! |
FYI you chatted on IRC, that's @matrixise pushed so many small commits quickly :-) |
* master: (621 commits) Update opcode.h header comment to mention the source data file (pythonGH-9935) bpo-34936: Fix TclError in tkinter.Spinbox.selection_element(). (pythonGH-9760) Updated documentation on logging.debug(). (pythonGH-9946) bpo-34765: Update the install-sh file (pythonGH-9592) bpo-35008: Fix possible leaks in Element.__setstate__(). (pythonGH-9924) bpo-35011: Restore use of pyexpatns.h in libexpat (pythonGH-9939) bpo-24658: Fix read/write greater than 2 GiB on macOS (pythonGH-1705) Add missing comma to wsgiref doc (pythonGH-9932) bpo-23420: Verify the value of '-s' when execute the CLI of cProfile (pythonGH-9925) Doc: Fix is_prime (pythonGH-9909) In email docs, correct spelling of foregoing (python#9856) In email.parser in message_from_bytes, update `strict` to `policy` (python#9854) bpo-34997: Fix test_logging.ConfigDictTest.test_out_of_order (pythonGH-9913) Added CLI starter example to logging cookbook. (pythonGH-9910) bpo-34783: Fix test_nonexisting_script() (pythonGH-9896) bpo-23554: Change echo server example class name from EchoServerClientProtocol to EchoServerProtocol (pythonGH-9859) bpo-34989: python-gdb.py: fix current_line_num() (pythonGH-9889) Stop using deprecated logging API in Sphinx suspicious checker (pythonGH-9875) fix dangling keyfunc examples in documentation of heapq and sorted (python#1432) bpo-34844: logging.Formatter enhancement - Ensure style and format string matches in logging.Formatter (pythonGH-9703) ...
* master: (1787 commits) Update opcode.h header comment to mention the source data file (pythonGH-9935) bpo-34936: Fix TclError in tkinter.Spinbox.selection_element(). (pythonGH-9760) Updated documentation on logging.debug(). (pythonGH-9946) bpo-34765: Update the install-sh file (pythonGH-9592) bpo-35008: Fix possible leaks in Element.__setstate__(). (pythonGH-9924) bpo-35011: Restore use of pyexpatns.h in libexpat (pythonGH-9939) bpo-24658: Fix read/write greater than 2 GiB on macOS (pythonGH-1705) Add missing comma to wsgiref doc (pythonGH-9932) bpo-23420: Verify the value of '-s' when execute the CLI of cProfile (pythonGH-9925) Doc: Fix is_prime (pythonGH-9909) In email docs, correct spelling of foregoing (python#9856) In email.parser in message_from_bytes, update `strict` to `policy` (python#9854) bpo-34997: Fix test_logging.ConfigDictTest.test_out_of_order (pythonGH-9913) Added CLI starter example to logging cookbook. (pythonGH-9910) bpo-34783: Fix test_nonexisting_script() (pythonGH-9896) bpo-23554: Change echo server example class name from EchoServerClientProtocol to EchoServerProtocol (pythonGH-9859) bpo-34989: python-gdb.py: fix current_line_num() (pythonGH-9889) Stop using deprecated logging API in Sphinx suspicious checker (pythonGH-9875) fix dangling keyfunc examples in documentation of heapq and sorted (python#1432) bpo-34844: logging.Formatter enhancement - Ensure style and format string matches in logging.Formatter (pythonGH-9703) ...
…-1705) On macOS, fix reading from and writing into a file with a size larger than 2 GiB.
On macOS, fix reading from and writing into a file with a size larger than 2 GiB.
https://bugs.python.org/issue24658