bpo-17852: Fix PR #3372, flush BufferedWriter objects on exit.

[nascheme]

New version of PR #3372, which was reverted by 317def9.

We can't use _Py_PyAtExit() as it only supports registering a single
callback. It is used by the atexit module and so we can't use it. We
can't use Py_AtExit() either because it calls functions too late in the
interpreter shutdown process. Instead, create io._flush_all_buffers.
In io.py, register it with the atexit module.

https://bugs.python.org/issue17852

[pitrou]

except Exception sounds better here.

[pitrou]

I think the approach here is nice and adequate. It would be nice to come up with a test case, if that's easily doable.

[bedevere-bot]

When you're done making the requested changes, leave the comment: I have made the requested changes; please review again.

[nascheme]

I have made the requested changes; please review again.

[bedevere-bot]

Thanks for making the requested changes!

@pitrou: please review the changes made to this pull request.

[nascheme]

I spent some time trying to think of how to build a test. It is difficult. I believe you need a BufferedWriter + FileIO pair to be part of a reference cycle. Then you need the finalization of the FileIO object to happen before the BufferedWriter. At least, that is my memory of how to trigger the non-flushing behaviour. I spent about half a day trying to trigger the bug originally and figure out what was happening. Unfortunately I did not save the script that triggered it. I do distinctly recall that the buffered file needs to be part of a reference cycle. The motivation for me to make this patch was to hopefully save someone else going through solving a similar bug.

[nascheme]

Ha ha, I managed to trigger it. I added a script to bpo-17852. Also added some explanation that this bug has to do with topologically ordering of finalizers.

[pitrou]

Could you convert your script into a simple unit test?

[nascheme]

Closing this PR as it is not really a fix. It works if the reference cycle containing the raw file and buffered file object are not re-claimed before atexit gets run. However, the cycle can be claimed during a normal gc.collect() run. In that case, the raw file can get closed before the buffered file and data in the buffer will be discarded.

I thought having the raw file keep a list of weak refs to the buffer and calling flush() on those when the raw close() is called would be a proper fix. However, that doesn't work either. The GC clears the weakrefs in handle_weakrefs() before it calls __del__ on the raw file object. So, the raw file cannot get the buffer to flush itself before the raw file is closed. So, using weak refs does not work, at least with how gcmodule works at the moment.

The only other idea I have is to split the buffered IO object into two parts. A "proxy" object that wraps the underlying state. The raw file object would keep a strong reference to this underlying state object.Then the raw file can call flush() before closing itself.