Steffen Daode Nurpmeso <sdao...@googlemail.com> added the comment:
On Thu, Apr 07, 2011 at 01:12:46AM +0000, R. David Murray wrote:
> [...] should be sufficient.
It is sufficient to fix the resource warning.
Having a completely dynamic language is a nice thing.
I would not do it like that.
Instead i would even consider to provide at least ProxyFile
publically (i.e. in I/O), because it is a nifty thing which may
be useful for a number of applications.
(PartialFile is probably too complicated in respect to SMP and FD
sharing to be public as a generic (non-abstract) class.)
I don't want to come up with a C++ lib again, but there Device-s
are doubly-linked (spinlock protected), and each has a list of
attached Stream-s, and upon program exit the list(s) is/are
walked:
open:
"IO::Device::open(): already open!!%@"
"\tI'll fail with Errno::ebusy now.%@"
close:
"IO::Device::close(): device *not* open!!%@"
"\tThis may break non-debug enabled code...%@"
"\tI'll fail with Errno::enodev now.%@"
"IO::Device::close(): error occurred while closing one%@"
"\tof the still attached streams!%@"
~Device:
"IO::Device::~Device(): still isOpen()!%@"
"\tEither subclass::close() does not call Device::close(),%@"
"\tor it's destructor does not check the open state.%@"
"\tI'll clean up anyway. Expect crashes soon.%@"
Stream somewhat likewhat. The Issues #11767, #11466, #11701, to
name a few, would never happen there if at least one test uses at
least one of the involved classes once.
Now this is not true for Python's I/O, but i think that the
interface should at least be complete and act as documented,
so saying "file-like object" implies raising of ValueError if
an object is closed etc.
Even if the class is a somewhat hidden implementation detail.
So of course hasattr() could be used instead of a special member
to test for is_open(). (I hope it's slower than the latter.)
There is no IOXY.open(), so, well, ok.
By the way, mailbox.py also has some other pitfalls as far as
i remember; i think i've even seen calls to _lock_file() for
a successful is_locked() (self._locked or so) state, which results
in unlocking because of cleanup due to lock-taking failure;
it was a shallow look only.
(That is, because: rescue me from here!!!)
I'll attach 11700.yeah.diff, which inherits from RawIOBase;
it's test adds some stuff naively.
Note this is a few-hours hack in an area i haven't touched that
much yet; it's a bit mysterious which I/O base class implements
which functions or expects which function to be present in
a subclass ... And which raises what and when.
That is - i would need to regulary review that at least once
before i would ship that out for real.
And i'll attach 11700.sigh.diff, which does only what is
necessary, following your suggestions.
Now this: choose the right diff and i'll implement #11783.
:)
----------
Added file: http://bugs.python.org/file21560/11700.yeah.diff
Added file: http://bugs.python.org/file21561/11700.sigh.diff
_______________________________________
Python tracker <rep...@bugs.python.org>
<http://bugs.python.org/issue11700>
_______________________________________diff --git a/Lib/mailbox.py b/Lib/mailbox.py
--- a/Lib/mailbox.py
+++ b/Lib/mailbox.py
@@ -1864,40 +1864,125 @@
"""Message with MMDF-specific properties."""
-class _ProxyFile:
- """A read-only wrapper of a file."""
+class _ProxyFile(io.RawIOBase):
+ """A io.RawIOBase inheriting read-only wrapper for a seekable file.
+ It supports __iter__() and the context-manager protocol.
+ """
+ def __init__(self, file, pos=None):
+ """If pos is not None then the file will keep track of its position."""
+ self._file = file
+ self._pos = pos
+ self._trackpos = True if pos is not None else False
+ self._close = True
+ self._is_open = True
- def __init__(self, f, pos=None):
- """Initialize a _ProxyFile."""
- self._file = f
- if pos is None:
- self._pos = f.tell()
+ def _set_noclose(self):
+ """Subclass hook - use to avoid closing internal file object."""
+ self._close = False
+
+ def _closed_check(self):
+ """Raise ValueError if not open."""
+ if not self._is_open:
+ raise ValueError('I/O operation on closed file')
+
+ def close(self):
+ if self._close:
+ self._close = False
+ self._file.close()
+ del self._file
+ self._is_open = False
+
+ @property
+ def closed(self):
+ return not self._is_open
+
+ def _write_check(self):
+ """Raises io.UnsupportedOperation."""
+ raise io.UnsupportedOperation('ProxyFile is readonly')
+
+ #def fileno(self):
+ def flush(self):
+ self._closed_check()
+ self._write_check()
+
+ #def isatty(self):
+
+ def _read(self, size, read_method, add_arg=None):
+ if size < 0:
+ size = -1
+ if self._trackpos:
+ self._file.seek(self._pos)
+ if not add_arg:
+ result = read_method(size)
else:
- self._pos = pos
+ result = read_method(size, add_arg)
+ if self._trackpos:
+ self._pos = self._file.tell()
+ return result
- def read(self, size=None):
- """Read bytes."""
- return self._read(size, self._file.read)
+ def readable(self):
+ self._closed_check()
+ return self._file.readable()
- def read1(self, size=None):
- """Read bytes."""
- return self._read(size, self._file.read1)
-
- def readline(self, size=None):
- """Read a line."""
+ def readline(self, size=-1):
+ self._closed_check()
return self._read(size, self._file.readline)
- def readlines(self, sizehint=None):
- """Read multiple lines."""
+ def readlines(self, sizehint=-1):
result = []
for line in self:
result.append(line)
- if sizehint is not None:
+ if sizehint >= 0:
sizehint -= len(line)
if sizehint <= 0:
break
return result
+ def read(self, size=-1):
+ self._closed_check()
+ return self._read(size, self._file.read)
+
+ #def readall(self):
+
+ def readinto(self, by_arr):
+ self._closed_check()
+ return self._read(len(by_arr), self._file.readinto, by_arr)
+
+ def seekable(self):
+ self._closed_check()
+ return True
+
+ def seek(self, offset, whence=0):
+ self._closed_check()
+ if whence == 1:
+ if not self._trackpos:
+ self._pos = self._file.tell()
+ self._file.seek(self._pos)
+ self._pos = self._file.seek(offset, whence)
+ return self._pos
+
+ def tell(self):
+ self._closed_check()
+ if not self._trackpos:
+ self._pos = self._file.tell()
+ return self._pos
+
+ def truncate(self, size=None):
+ self._closed_check()
+ self._write_check()
+
+ def writable(self):
+ self._closed_check()
+ return False
+
+ def writelines(self, lines):
+ self._closed_check()
+ self._write_check()
+
+ def write(self, by_arr):
+ self._closed_check()
+ self._write_check()
+
def __iter__(self):
"""Iterate over lines."""
while True:
@@ -1906,32 +1991,6 @@
raise StopIteration
yield line
- def tell(self):
- """Return the position."""
- return self._pos
-
- def seek(self, offset, whence=0):
- """Change position."""
- if whence == 1:
- self._file.seek(self._pos)
- self._file.seek(offset, whence)
- self._pos = self._file.tell()
-
- def close(self):
- """Close the file."""
- if hasattr(self._file, 'close'):
- self._file.close()
- del self._file
-
- def _read(self, size, read_method):
- """Read size bytes using read_method."""
- if size is None:
- size = -1
- self._file.seek(self._pos)
- result = read_method(size)
- self._pos = self._file.tell()
- return result
-
def __enter__(self):
"""Context manager protocol support."""
return self
@@ -1939,29 +1998,16 @@
def __exit__(self, *exc):
self.close()
- def readable(self):
- return self._file.readable()
-
- def writable(self):
- return self._file.writable()
-
- def seekable(self):
- return self._file.seekable()
-
- def flush(self):
- return self._file.flush()
-
- @property
- def closed(self):
- return self._file.closed
-
class _PartialFile(_ProxyFile):
"""A read-only wrapper of part of a file."""
def __init__(self, f, start=None, stop=None):
"""Initialize a _PartialFile."""
+ if start is None:
+ start = f.tell()
_ProxyFile.__init__(self, f, start)
+ super()._set_noclose()
self._start = start
self._stop = stop
@@ -1971,6 +2017,7 @@
def seek(self, offset, whence=0):
"""Change position, possibly with respect to start or stop."""
+ self._closed_check()
if whence == 0:
self._pos = self._start
whence = 1
@@ -1988,11 +2035,6 @@
size = remaining
return _ProxyFile._read(self, size, read_method)
- def close(self):
- # do *not* close the underlying file object for partial files,
- # since it's global to the mailbox object
- del self._file
-
def _lock_file(f, dotlock=True):
"""Lock file f using lockf and dot locking."""
diff --git a/Lib/test/test_mailbox.py b/Lib/test/test_mailbox.py
--- a/Lib/test/test_mailbox.py
+++ b/Lib/test/test_mailbox.py
@@ -290,12 +290,14 @@
key1 = self._box.add(_sample_message)
with self._box.get_file(key0) as file:
data0 = file.read()
- with self._box.get_file(key1) as file:
- data1 = file.read()
+ file1 = self._box.get_file(key1)
+ data1 = file1.read()
self.assertEqual(data0.decode('ascii').replace(os.linesep, '\n'),
self._template % 0)
self.assertEqual(data1.decode('ascii').replace(os.linesep, '\n'),
_sample_message)
+ file1.close()
+ file1.close()
def test_iterkeys(self):
# Get keys using iterkeys()
@@ -1833,10 +1835,35 @@
self.assertFalse(proxy.read())
def _test_close(self, proxy):
- # Close a file
+ self.assertFalse(proxy.closed)
+ self.assertTrue(proxy.seekable())
+ self.assertRaises(io.UnsupportedOperation, proxy.flush)
+ self.assertRaises(io.UnsupportedOperation, proxy.truncate, 0)
+ self.assertFalse(proxy.writable())
+ self.assertRaises(io.UnsupportedOperation, proxy.writelines, ['AU'])
+ self.assertRaises(io.UnsupportedOperation, proxy.write, 'AU')
+
proxy.close()
- self.assertRaises(AttributeError, lambda: proxy.close())
+ self.assertTrue(proxy.closed)
+ try:
+ proxy.close()
+ except:
+ self.fail('Proxy.close() failure')
+ self.assertRaises(ValueError, proxy.flush)
+ self.assertRaises(ValueError, proxy.readable)
+ self.assertRaises(ValueError, proxy.readline)
+ self.assertRaises(ValueError, proxy.readlines)
+ self.assertRaises(ValueError, proxy.read)
+ self.assertRaises(ValueError, proxy.readall)
+ self.assertRaises(ValueError, proxy.readinto, bytearray())
+ self.assertRaises(ValueError, proxy.seekable)
+ self.assertRaises(ValueError, proxy.seek, 0)
+ self.assertRaises(ValueError, proxy.tell)
+ self.assertRaises(ValueError, proxy.truncate)
+ self.assertRaises(ValueError, proxy.writable)
+ self.assertRaises(ValueError, proxy.writelines, ['AU'])
+ self.assertRaises(ValueError, proxy.write, 'AU')
class TestProxyFile(TestProxyFileBase):
diff --git a/Lib/mailbox.py b/Lib/mailbox.py
--- a/Lib/mailbox.py
+++ b/Lib/mailbox.py
@@ -1919,9 +1919,9 @@
def close(self):
"""Close the file."""
- if hasattr(self._file, 'close'):
+ if hasattr(self, '_file'):
self._file.close()
- del self._file
+ del self._file
def _read(self, size, read_method):
"""Read size bytes using read_method."""
@@ -1953,7 +1953,7 @@
@property
def closed(self):
- return self._file.closed
+ return not hasattr(self, '_file')
class _PartialFile(_ProxyFile):
@@ -1991,7 +1991,8 @@
def close(self):
# do *not* close the underlying file object for partial files,
# since it's global to the mailbox object
- del self._file
+ if hasattr(self, '_file'):
+ del self._file
def _lock_file(f, dotlock=True):
diff --git a/Lib/test/test_mailbox.py b/Lib/test/test_mailbox.py
--- a/Lib/test/test_mailbox.py
+++ b/Lib/test/test_mailbox.py
@@ -1834,9 +1834,13 @@
def _test_close(self, proxy):
# Close a file
+ self.assertFalse(proxy.closed)
proxy.close()
- self.assertRaises(AttributeError, lambda: proxy.close())
-
+ self.assertTrue(proxy.closed)
+ try:
+ proxy.close()
+ except:
+ self.fail('Proxy.close() failure')
class TestProxyFile(TestProxyFileBase):
_______________________________________________
Python-bugs-list mailing list
Unsubscribe:
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
