classification
Title: mmap.flush does not check for errors on windows
Type: behavior Stage: patch review
Components: Library (Lib) Versions: Python 3.3, Python 3.2, Python 2.7
process
Status: open Resolution:
Dependencies: Superseder:
Assigned To: brian.curtin Nosy List: brian.curtin, ishimoto, ocean-city, pitrou, schmir, tim.golden
Priority: high Keywords: patch

Created on 2008-02-15 13:20 by schmir, last changed 2012-07-25 03:18 by ishimoto.

Files
File name Uploaded Description Edit
mmap-flush.txt schmir, 2008-03-10 08:19 make flush return None
Messages (6)
msg62427 - (view) Author: Ralf Schmitt (schmir) Date: 2008-02-15 13:20
mmap.flush returns the result of the call to FlushViewOfFile as an
integer, and does not check for errors. On unix it does check for
errors. The function should return None and raise an exception if an
error occurs...
This bug can lead to data loss...

Here's the current version of that function:

static PyObject *
mmap_flush_method(mmap_object *self, PyObject *args)
{
	Py_ssize_t offset = 0;
	Py_ssize_t size = self->size;
	CHECK_VALID(NULL);
	if (!PyArg_ParseTuple(args, "|nn:flush", &offset, &size))
		return NULL;
	if ((size_t)(offset + size) > self->size) {
		PyErr_SetString(PyExc_ValueError, "flush values out of range");
		return NULL;
	}
#ifdef MS_WINDOWS
	return PyInt_FromLong((long) FlushViewOfFile(self->data+offset, size));
#elif defined(UNIX)
	/* XXX semantics of return value? */
	/* XXX flags for msync? */
	if (-1 == msync(self->data + offset, size, MS_SYNC)) {
		PyErr_SetFromErrno(mmap_module_error);
		return NULL;
	}
	return PyInt_FromLong(0);
#else
	PyErr_SetString(PyExc_ValueError, "flush not supported on this system");
	return NULL;
#endif
}
msg63436 - (view) Author: Ralf Schmitt (schmir) Date: 2008-03-10 08:19
attached is a patch. I hope it is ok to change the semantics a bit. 
They were very strange:
- on unix it raises an exception on errors. otherwise it always returns 0.
- on windows it returns non-zero on success, and returns zero on errors.

Now, flush returns None or raises an Exception.
msg65549 - (view) Author: Ralf Schmitt (schmir) Date: 2008-04-16 13:21
now this insanity is even documented with svn revision 62359
(http://hgpy.de/py/trunk/rev/442252bce780)
msg65550 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2008-04-16 16:02
Perhaps it would be better if the patch came with unit tests. I agree
that the situation right now is insane.

PS : I have no power to commit or even accept a patch :)
msg65551 - (view) Author: Ralf Schmitt (schmir) Date: 2008-04-16 16:26
I thought about this too, but I don't know of a way to provoke an error.
(Other than passing in illegal values, but the code tries to catch
those. And btw, raises an Exception on windows :)

One could currently pass in a negative value for size (this isn't caught
in the checks). e.g.

m.flush(500, -500) gives an 'error: [Errno 22] Invalid argument' on linux.

but then you don't want to rely on another bug for testing purposes.
msg138227 - (view) Author: Hirokazu Yamamoto (ocean-city) * (Python committer) Date: 2011-06-13 03:00
This issue seems to be reproduced in following way.

1. Attach USB flash drive. (On my machine, it was attached as E drive)

2. Run python interactive shell and run following commands.
   (Confirmed on Python2.6)

  > import mmap
  > f = open("e:/temp.tmp", "w+b")
  > f.write("foo")
  > f.flush()
  > m = mmap.mmap(f.fileno(), 3)
  > m[:] = "xxx"

3. Detach USB flash drive violently here! (Without safety mechanism
   to detach USB flash drive, just plug it off) Note that
   python shell is still running.

4. Enter following command in python shell.
  > m.flush()
   It returns *0*. (Means failure)
History
Date User Action Args
2012-07-25 03:18:45ishimotosetnosy: + ishimoto
2011-06-13 11:17:41pitrousetstage: test needed -> patch review
2011-06-13 03:00:19ocean-citysetmessages: + msg138227
2011-06-12 18:50:10terry.reedysetversions: + Python 3.3, - Python 3.1
2010-09-03 23:48:03brian.curtinsetassignee: brian.curtin
2010-09-03 23:23:15pitrousetnosy: + tim.golden, brian.curtin

versions: - Python 2.6
2010-05-11 20:56:40terry.reedysetversions: + Python 2.7, Python 3.2
2009-05-16 19:42:55pitrousetnosy: + ocean-city
2009-05-16 19:37:56ajaksu2setpriority: high
stage: test needed
versions: + Python 3.1, - Python 2.5, Python 3.0
2008-04-16 16:26:17schmirsetmessages: + msg65551
2008-04-16 16:02:58pitrousetnosy: + pitrou
messages: + msg65550
2008-04-16 13:21:02schmirsetmessages: + msg65549
2008-04-15 05:50:28loewissetkeywords: + patch
2008-03-10 08:19:23schmirsetfiles: + mmap-flush.txt
messages: + msg63436
2008-02-15 13:20:02schmircreate