classification
Title: mmap.flush() on Linux does not accept the "offset" and "size" args
Type: behavior Stage: patch review
Components: Documentation Versions: Python 3.8, Python 3.7, Python 3.6, Python 2.7
process
Status: open Resolution:
Dependencies: Superseder:
Assigned To: docs@python Nosy List: berker.peksag, byronhawkins, christian.heimes, docs@python, pablogsal, socketpair, xiang.zhang
Priority: low Keywords: easy, patch

Created on 2018-02-08 15:06 by byronhawkins, last changed 2018-03-29 00:32 by berker.peksag.

Pull Requests
URL Status Linked Edit
PR 5621 open pablogsal, 2018-02-11 15:21
Messages (7)
msg311832 - (view) Author: Byron Hawkins (byronhawkins) Date: 2018-02-08 15:06
open_file = open("file.txt", "r+b")
file_map = mmap.mmap(open_file, 0)
file_map.seek(offset)
file_map.write("foobar") # success
file_map.flush() # success
file_map.flush(offset, len("foobar")) # Fails with "errno 22"

This example passes correct arguments to mmap.flush(), yet it fails with errno 22. So the arguments are not valid on linux. If the bug cannot be fixed, then all reference to those two arguments should be removed from the code and documentation related to Linux.
msg311833 - (view) Author: Christian Heimes (christian.heimes) * (Python committer) Date: 2018-02-08 15:11
offset must be a multiple of pagesize. This is a known but undocumented restriction on some platforms, see http://man7.org/linux/man-pages/man2/msync.2.html
msg311834 - (view) Author: Christian Heimes (christian.heimes) * (Python committer) Date: 2018-02-08 15:14
I'm marking this as a low priority and simple documentation bug. The mmap module exposes the page size of the architecture as mmap.PAGESIZE.
msg311835 - (view) Author: Byron Hawkins (byronhawkins) Date: 2018-02-08 15:22
Couldn't the implementation check the page size and throw a better error (for relevant platforms)? Or better yet, adjust the offset to the nearest inclusive page boundary.
msg311882 - (view) Author: Марк Коренберг (socketpair) * Date: 2018-02-09 12:48
Actually documented: 

http://man7.org/linux/man-pages/man2/msync.2.html

EINVAL: addr is not a multiple of PAGESIZE;
msg312098 - (view) Author: Xiang Zhang (xiang.zhang) * (Python committer) Date: 2018-02-13 03:32
One thing to note is the behaviour seems fit implementation detail. POSIX doesn't requires this for both mmap and msync, it's optional:

The mmap( ) function may fail if:
[EINVAL] The addr argument (if MAP_FIXED was specified) or off is not a multiple of the page size as returned

The msync( ) function may fail if:
[EINVAL] The value of addr is not a multiple of the page size as returned by sysconf( ).
msg314626 - (view) Author: Berker Peksag (berker.peksag) * (Python committer) Date: 2018-03-29 00:32
By the way, there is a note in the "Change History" section of the msync() documentation at http://pubs.opengroup.org/onlinepubs/9699919799/

    The second [EINVAL] error condition is made mandatory.

The second EINVAL error condition they mention is:

    [EINVAL] The value of addr is not a multiple of the page size as returned by sysconf().

So unless I'm missing something obvious, the "the msync() function may fail if" part may now be outdated?

Also, I noticed that we don't have tests for mm.flush(offset) and mm.flush(offset, size) cases. It would be nice to add some tests even if we decide to not merge the documentation PR.
History
Date User Action Args
2018-03-29 00:32:01berker.peksagsetmessages: + msg314626
2018-02-13 03:32:56xiang.zhangsetnosy: + pablogsal, berker.peksag, xiang.zhang
messages: + msg312098
2018-02-11 15:21:46pablogsalsetkeywords: + patch
stage: patch review
pull_requests: + pull_request5430
2018-02-09 12:48:23socketpairsetnosy: + socketpair
messages: + msg311882
2018-02-08 15:22:34byronhawkinssetmessages: + msg311835
2018-02-08 15:14:12christian.heimessetkeywords: + easy

messages: + msg311834
2018-02-08 15:11:03christian.heimessetpriority: normal -> low

type: crash -> behavior
assignee: docs@python
components: + Documentation, - Library (Lib)
versions: + Python 3.6, Python 3.7, Python 3.8
nosy: + docs@python, christian.heimes

messages: + msg311833
2018-02-08 15:06:58byronhawkinscreate