$ ./python
Python 3.4.0b1 (default:acabd3f035fe, Nov 28 2013, 15:04:09) 
[GCC 4.8.2 20131017 (Red Hat 4.8.2-1)] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import _pyio
>>> f=_pyio.open("/etc/issue"); f=None
>>> f=_pyio.open("/etc/issue", "rb"); f=None
>>> f=_pyio.open("/etc/issue", "rb", 0); f=None
__main__:1: ResourceWarning: unclosed file <_io.FileIO name='/etc/issue' mode='rb'>
>>> import io
>>> f=io.open("/etc/issue"); f=None
__main__:1: ResourceWarning: unclosed file <_io.TextIOWrapper name='/etc/issue' mode='r' encoding='UTF-8'>
>>> f=io.open("/etc/issue", "rb"); f=None
__main__:1: ResourceWarning: unclosed file <_io.BufferedReader name='/etc/issue'>
>>> f=io.open("/etc/issue", "rb", 0); f=None
__main__:1: ResourceWarning: unclosed file <_io.FileIO name='/etc/issue' mode='rb'>

I expect the same behaviour when I use _pyio or io module.

I think it will be good.

Related issue21859 "Add Python implementation of FileIO"

It looks like the C _io close() implementations delegate to the wrapped object’s undocumented _dealloc_warn() method to emit the warning if “self->finalizing” is set. For wrapped objects like BytesIO that do not have it, I guess the error due to the missing method is ignored.

There is no Python implementation of FileIO yet (see Issue 21859), so the Python file wrapper classes still use the C FileIO implementation, which emits a resource warning. However for the wrapper classes, _pyio.IOBase.__del__() explicitly invokes the wrapper’s close() method, which will invoke FileIO.close() method, which bypasses the warning.

Similarly, any object inheriting from the C implmentation’s _io.IOBase will also have close() explicitly invoked when being finalized, bypassing any warnings of the underlying wrapped object. The SocketIO instances returned by socket.makefile() are an example of this.

One option would be for any wrapper-type class (e.g. BufferedReader, SocketIO) to override __del__(). Instead of calling close(), it should call detach() or equivalent, and delete the returned reference to the underlying wrapped object without explicitly closing it, to allow a warning to be generated. Maybe a WrappedIO mixin class could help with this.

I am posting del-detach.patch which roughly implements the above option. It only seems to break the test_override_destructor() tests for the TextIOWrapperTest and CommonBufferedTests classes, which expect close() to be called.

Another more radical option might be to make IOBase.__del__() only call flush(), not close(), except for the actual implementations that really should emit a warning, like FileIO. I will have a go at doing this option in another patch.

Both these options would change the behaviour in some cases that rely on the garbage collector to close objects. They would defer the automatic closing until all references of the underlying object are garbage collected, instead of just when the wrapped object is garbage collected. A third option would be to add explicit __del__() methods emitting a warning in each class, along with any exception supression needed due to Python interpreter finalization quirks.

Posting del-flush.patch, which only calls flush() instead of close() from __del__(), except for the FileIO class. Quick analysis of resulting test failures:

These tests fail because they are obviously written to test that __del__ calls close():
* IOTest.test_IOBase_destructor
* IOTest.test_RawIOBase_destructor
* IOTest.test_BufferedIOBase_destructor
* IOTest.test_TextIOBase_destructor
* test_override_destructor as mentioned above

CBufferedReader/Writer/RandomTest.test_garbage_collection() fail because they do not free the reference to the wrapped raw file object. Similarly, CTextIOWrapperTest.test_garbage_collection() holds references to both levels of wrapped object.

TextIOWrapperTest.test_destructor() fails because it assumes the wrapped close() will be called. The data is still written to the wrapped file though.

MiscIOTest.test_warn_on_dealloc/_fd() fail because the warning message only refers to the FileIO object rather than the wrapper object.

Attached patch modifies _pyio to mimick better the reference io module:

* Add IOBase._finalizing
* IOBase.__del__() sets _finalizing to True
* Add FileIO._dealloc_warn()
* Remove FileIO.__del__()
* FileIO.close(), _BufferedIOMixin.close() and TextIOWrapper.close() now calls _dealloc_warn() if _finalizing is true
* Override closed() method in FileIO: needed because FileIO.close() now calls super().close() before logging the ResourceWarning, and the file is expected to see open in the warning
* FileIO.close() now calls super().close() *before* closing the file descriptor

I added hasattr(.., '_dealloc_warn') sanity checks because _dealloc_warn() are added to concrete classes, not to base classes.

(oops, there was a typo in my first patch)

Oh, the io module is more complex than what I expected :-) test_io fails with pyio_res_warn-2.patch.

test_io now pass with pyio_res_warn-3.patch. I fixed FileIO.close() to always call super().close(), and set self._fd to -1 even if logging the ResourceWarning raised an exception.

Try attached res_warn.py script to test manually all ResourceWarning warnings.

Ouput of python3.6 -Wd res_warn.py:
--------------------------
text
<_pyio.TextIOWrapper name='/etc/issue' mode='r' encoding='UTF-8'>
res_warn.py:9: ResourceWarning: unclosed file <_pyio.FileIO name='/etc/issue' mode='rb' closefd=True>
  f = None

buffered
<_pyio.BufferedReader name='/etc/issue'>
res_warn.py:15: ResourceWarning: unclosed file <_pyio.FileIO name='/etc/issue' mode='rb' closefd=True>
  f=None

raw
<_pyio.FileIO name='/etc/issue' mode='rb' closefd=True>
res_warn.py:21: ResourceWarning: unclosed file <_pyio.FileIO name='/etc/issue' mode='rb' closefd=True>
  f=None

fileio
<_pyio.FileIO name='/etc/issue' mode='rb' closefd=True>
res_warn.py:27: ResourceWarning: unclosed file <_pyio.FileIO name='/etc/issue' mode='rb' closefd=True>
  f=None

--------------------------


Ouput of python3.6 -X tracemalloc=2 -Wd res_warn.py:
--------------------------
text
<_pyio.TextIOWrapper name='/etc/issue' mode='r' encoding='UTF-8'>
res_warn.py:9: ResourceWarning: unclosed file <_pyio.FileIO name='/etc/issue' mode='rb' closefd=True>
  f = None
Object allocated at (most recent call first):
  File "/home/haypo/prog/python/default/Lib/_pyio.py", lineno 209
    closefd, opener=opener)
  File "res_warn.py", lineno 7
    f = _pyio.open(fn, "r")

buffered
<_pyio.BufferedReader name='/etc/issue'>
res_warn.py:15: ResourceWarning: unclosed file <_pyio.FileIO name='/etc/issue' mode='rb' closefd=True>
  f=None
Object allocated at (most recent call first):
  File "/home/haypo/prog/python/default/Lib/_pyio.py", lineno 209
    closefd, opener=opener)
  File "res_warn.py", lineno 13
    f = _pyio.open(fn, "rb")

raw
<_pyio.FileIO name='/etc/issue' mode='rb' closefd=True>
res_warn.py:21: ResourceWarning: unclosed file <_pyio.FileIO name='/etc/issue' mode='rb' closefd=True>
  f=None
Object allocated at (most recent call first):
  File "/home/haypo/prog/python/default/Lib/_pyio.py", lineno 209
    closefd, opener=opener)
  File "res_warn.py", lineno 19
    f = _pyio.open(fn, "rb", 0)

fileio
<_pyio.FileIO name='/etc/issue' mode='rb' closefd=True>
res_warn.py:27: ResourceWarning: unclosed file <_pyio.FileIO name='/etc/issue' mode='rb' closefd=True>
  f=None
Object allocated at (most recent call first):
  File "res_warn.py", lineno 25
    f = _pyio.FileIO(fn, "rb")

--------------------------

del-detach.patch: I don't understand the change in _pyio.py. I think that the change on socket.py is no more need since the change 46329eec5515 (issue #26590).

del-flush.patch:
* change on IOBase.__del__() change the behaviour compared to the io module, I don't think that it's correct. I may also break backward compatibility :-/
* change on fileio.c implements a finalizer for FileIO: io.IOBase already implements a finalizer which calls close(), I'm not sure that this change is needed?
* change on iobase.c removes io.IOBase finalizer. I don't understand this. It breaks ResourceWarning on io.Buffered* and io.TextIOWrapper classes, no?

Well, I guess that these two patches were written before Serhiy reimplemented the FileIO in pure Python, and this change outdated your two patches.

I don't like _dealloc_warn() at all. It looks as a dirty hack and doesn't work with custom file-like objects.

I think we should use one of Martin's option. If there are any issues with garbage collecting, we should fix the garbage collector.

> I don't like _dealloc_warn() at all. It looks as a dirty hack and doesn't work with custom file-like objects.

I tried to support emitting a ResourceWarning when the close() method is overriden in a subclass, but then I saw that io doesn't support this use case neither:
---
import io
import os

class MyFile(io.FileIO):
    def close(self):
        os.close(self.fileno())

f = MyFile('/etc/issue')
f = None
---
=> no warning

Ok, now I have to read again Martin's patches since I didn't understand them at the first read :-)

> I think we should use one of Martin's option. If there are any issues with garbage collecting, we should fix the garbage collector.

Ok, here is a new simpler proposition: remove all _dealloc_warn() method, and simply emit the ResourceWarning in IOBase finalizer.

With this change, all objects which inherit from io.IOBase (or _pyio.IOBase) now emits a ResourceWarning if they are not closed explicitly. You have to override the __del__() method to prevent this warning.

A lot of new objects start to log ResourceWarning: io.SocketIO, _pyio.TextIOWrapper, io.BytesIO, http.client.HTTPResponse, etc.

For io.BytesIO, I fixed the code to inherit correctly IOBase finalizer.

--

A lot of tests start to fail because they emit a lot of ResourceWarning warnings. I don't know yet if it's a feature or a bug :-)

--

With the patch, Python starts to logs warnings like:

sys:1: ResourceWarning: unclosed file <_io.TextIOWrapper name='<stdout>' mode='w' encoding='UTF-8'>
sys:1: ResourceWarning: unclosed file <_io.TextIOWrapper name='<stdin>' mode='r' encoding='UTF-8'>

You can use my patch of the issue #26642 to fix these warnings.

ResourceWarning is emited even if FileIO.closefd is false.

Note: It looks like the ResourceWarning is not always displayed. Yet another bug?

> For io.BytesIO, I fixed the code to inherit correctly IOBase finalizer.

Oh, I forgot to include it in my patch. But it's maybe better to have it in a different patch: see attached bytesio_stringio.patch which changes also io.StringIO to inherit the IOBase finalizer.

Emitting a ResourceWarning in io.BytesIO and io.StringIO adds a *lot* of warnings when running the Python test suite. Since these objects only contain memory, no very limited resource like file descriptors, it's probably better to *not* log ResourceWarning for them.

Ok, I now understand the problem better. They are two kinds of io objects:

(1) object which directly or indirectly owns a limited resource like file descriptor => must emit a ResourceWarning
(2) object which don't own a limited resource => no ResourceWarning must be logged

Examples of (1): FileIO, BufferedReader(FileIO), TextIOWrapper(BufferedReader(FileIO))

Examples of (2): BytesIO, BuffereadReader(BytesIO), TextIOWrapper(BuffereadReader(BytesIO))


The tricky part is to decide if an object owns a limited resource or not. Currently, the io module uses the _dealloc_warn() trick. A close() method tries to call _dealloc_warn(), but it ignores any exception when calling _dealloc_warn(). BufferedReader calls raw._dealloc_warn(). TextIOWrapper calls buffer._dealloc_warn().


For case (1), BufferedReader(FileIO).close() calls FileIO._dealloc_warn() => ResourceWarning is logged

For case (2), BufferedReader(BytesIO).close() calls BytesIO._dealloc_warn() raises an AttributeError => no warning is logged


Well, we can call this a hack, but it works :-) pyio_res_warn-3.patch implements the same hack in _pyio.

1. What if only object which directly owns a limited resource will emit a ResourceWarning? This can break some tests, but may be these tests are too strong? Current GC may be better than when io classes were implemented.

2. An object which indirectly owns a limited resource don't know about this if use only public API. It can known about this only if the wrapped object provides _dealloc_warn(). Thus _dealloc_warn() extends the API. If allows an object which indirectly owns a limited resource emit a resource warning, it may be worth to expose _dealloc_warn() or something equivalent as public API.

In the long term, I prefer not calling close() in __del__(), like del-flush.patch, although I admit it is an API change and should probably be limited to 3.6+. If people want to improve things in 3.5, privately implementing _dealloc_warn() like Victor’s pyio_res_warn-3.patch seems the best option.

Serhiy: why did you add 2.7 to this bug? For 2.7, I don’t think anything should be done. There is no ResourceWarning in 2.7.

In 3.5, _dealloc_warn() could also be implemented in SocketIO (possibly also HTTPResponse, GzipFile, etc). But it should not become a public API, and I don’t think it is important to make this sort of change to 3.5 anyway.

In 3.6, if we stopped __del__() from calling close() like del-flush.patch, we would be changing the documented behaviour: <https://docs.python.org/3.6/library/io.html#io.IOBase.__del__> says “IOBase . . . calls the instance’s close() method.” But the end result seems cleaner to me. I think changing or adding an API (__del__, _dealloc_warn, etc) is necessary for a general solution to the problem.

The effect of del-flush.patch will be that a wrapped FileIO or similar object will not be closed until all references are deleted.

>>> file = open(os.devnull, "wb", 0)
>>> print(file)
<_io.FileIO name='/dev/null' mode='wb' closefd=True>
>>> wrapper = BufferedWriter(file)

In 3.5, deleting the wrapper produces a warning and closes the underlying file:

>>> del wrapper
__main__:1: ResourceWarning: unclosed file <_io.BufferedWriter name='/dev/null'>
>>> print(file)
<_io.FileIO [closed]>
>>> file.write(b"more data")
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
ValueError: I/O operation on closed file

I propose that in 3.6, deleting a wrapper should not automatically close or warn about the wrapped object until all references are deleted:

>>> del wrapper  # No problem; we still have a reference to "file"
>>> file.write(b"more data")  # File is still open
9
>>> del file  # Finally closes the file and triggers the warning
ResourceWarning: unclosed file <FileIO>

> Serhiy: why did you add 2.7 to this bug? For 2.7, I don’t think anything should be done. There is no ResourceWarning in 2.7.

Just for the case.

Not calling close() in __del__() is one option, and it looks attractive. But there are possible hidden catches. There is no guarantee that flush() writes all buffered data, some stateful encoder or compressor can left some data in the buffer unless explicitly closed. close() can write required footer or close connection. If not call close() in __del__(), GzipFile will produce incorrect file.

May be this is appropriate. This is why explicit close() should be called or context manager should be used.

Other option is to use some API through all closable objects. _dealloc_warn() is not the only option.

1. Add an optional parameter to close() methods to denote that it is called from __del__. This is backward incompatible option, it is too late to do this.

2. __del__() calls special method instead of close() if exists.

3. The _dealloc_warn() option (the name should be changed). The wrapper delegates emitting a warning to wrapped object by calling the _dealloc_warn() method if exists. Unlike to previous option this method is called in addition to close(), not instead.

4. The wrapper checks special attribute (or calls a method) of wrapped object. If it exists and is true, the wrapper emits a warning.

While it would be great to also emit ResourceWarning, in the last 8 years, nobody managed to implement the feature. Also, this issue has no activity. I prefer to close the issue.