[Python-checkins] r68372 - in sandbox/trunk/io-c: _bufferedio.c _iobase.c test_io.py

[antoine.pitrou]

Author: antoine.pitrou
Date: Wed Jan  7 02:03:52 2009
New Revision: 68372

Log:
Ensure we can't use a closed file as a context manager.
This fixes one test in test_tempfile.



Modified:
   sandbox/trunk/io-c/_bufferedio.c
   sandbox/trunk/io-c/_iobase.c
   sandbox/trunk/io-c/test_io.py

Modified: sandbox/trunk/io-c/_bufferedio.c
==============================================================================
--- sandbox/trunk/io-c/_bufferedio.c	(original)
+++ sandbox/trunk/io-c/_bufferedio.c	Wed Jan  7 02:03:52 2009
@@ -333,22 +333,36 @@
 static PyObject *
 BufferedIOMixin_close(BufferedObject *self, PyObject *args)
 {
-    PyObject *res;
+    PyObject *res = NULL;
+    int r;
 
     CHECK_INITIALIZED(self)
-    if (BufferedIOMixin_closed(self))
-        Py_RETURN_NONE;
+    ENTER_BUFFERED(self)
 
+    r = BufferedIOMixin_closed(self);
+    if (r < 0)
+        goto end;
+    if (r > 0) {
+        res = Py_None;
+        Py_INCREF(res);
+        goto end;
+    }
+    /* flush() will most probably re-take the lock, so drop it first */
+    LEAVE_BUFFERED(self)
     res = PyObject_CallMethodObjArgs((PyObject *)self, _PyIO_str_flush, NULL);
+    ENTER_BUFFERED(self)
     if (res == NULL) {
         /* If flush() fails, just give up */
         if (PyErr_ExceptionMatches(PyExc_IOError))
             PyErr_Clear();
         else
-            return NULL;
+            goto end;
     }
+    res = PyObject_CallMethodObjArgs(self->raw, _PyIO_str_close, NULL);
 
-    return PyObject_CallMethod(self->raw, "close", NULL);
+end:
+    LEAVE_BUFFERED(self)
+    return res;
 }
 
 /* Inquiries */

Modified: sandbox/trunk/io-c/_iobase.c
==============================================================================
--- sandbox/trunk/io-c/_iobase.c	(original)
+++ sandbox/trunk/io-c/_iobase.c	Wed Jan  7 02:03:52 2009
@@ -36,6 +36,13 @@
     "with open('spam.txt', 'r') as fp:\n"
     "    fp.write('Spam and eggs!')\n");
 
+/* Use this macro whenever you want to check the internal `closed` status
+   of the IOBase object rather than the virtual `closed` attribute as returned
+   by whatever subclass. */
+
+#define IS_CLOSED(self) \
+    PyObject_HasAttrString(self, "__IOBase_closed")
+
 /* Internal methods */
 static PyObject *
 IOBase_unsupported(const char *message)
@@ -108,13 +115,20 @@
 static int
 IOBase_closed(PyObject *self)
 {
-    return PyObject_HasAttrString(self, "__IOBase_closed");
+    PyObject *res;
+    int closed;
+    /* This gets the derived attribute, which is *not* __IOBase_closed
+       in most cases! */
+    res = PyObject_GetAttr(self, _PyIO_str_closed);
+    closed = PyObject_IsTrue(res);
+    Py_DECREF(res);
+    return closed;
 }
 
 static PyObject *
 IOBase_closed_get(PyObject *self, void *context)
 {
-    return PyBool_FromLong(IOBase_closed(self));
+    return PyBool_FromLong(IS_CLOSED(self));
 }
 
 
@@ -128,17 +142,20 @@
     Py_RETURN_NONE;
 }
 
+/* XXX: IOBase thinks it has to maintain its own internal state in
+   `__IOBase_closed` and call flush() by itself, but it is redundant with
+   whatever behaviour a non-trivial derived class will implement. */
+
 static PyObject *
 IOBase_close(PyObject *self, PyObject *args)
 {
     PyObject *res;
 
-    if (IOBase_closed(self))
+    if (IS_CLOSED(self))
         Py_RETURN_NONE;
 
-    PyObject_SetAttrString(self, "__IOBase_closed", Py_True);
-
     res = PyObject_CallMethodObjArgs(self, _PyIO_str_flush, NULL);
+    PyObject_SetAttrString(self, "__IOBase_closed", Py_True);
     if (res == NULL) {
         /* If flush() fails, just give up */
         if (PyErr_ExceptionMatches(PyExc_IOError))

Modified: sandbox/trunk/io-c/test_io.py
==============================================================================
--- sandbox/trunk/io-c/test_io.py	(original)
+++ sandbox/trunk/io-c/test_io.py	Wed Jan  7 02:03:52 2009
@@ -450,6 +450,18 @@
             self.assertEqual(record, [1, 2, 3])
         else:
             self.assertEqual(record, [1, 2])
+    
+    def testContext(self):
+        # Test usability as a context manager
+        rawio = MockRawIO()
+        bufio = self.tp(rawio)
+        def _with():
+            with bufio:
+                pass
+        _with()
+        # bufio should now be closed, and using it a second time should raise
+        # a ValueError.
+        self.assertRaises(ValueError, _with)
 
 class BufferedReaderTest(unittest.TestCase, CommonBufferedTests):
     tp = io.BufferedReader