classification
Title: asyncio _UnixWritePipeTransport._close abandons unflushed writes
Type: behavior Stage:
Components: asyncio Versions: Python 3.5
process
Status: open Resolution:
Dependencies: Superseder:
Assigned To: Nosy List: Robert Smallshire, gvanrossum, vstinner, yselivanov
Priority: normal Keywords:

Created on 2016-03-29 10:54 by Robert Smallshire, last changed 2016-03-29 10:54 by Robert Smallshire.

Messages (1)
msg262601 - (view) Author: Robert Smallshire (Robert Smallshire) Date: 2016-03-29 10:54
Note: This report is based on a close reading of the asyncio code, rather than having experienced defective behaviour in test or production.

The documentation for BaseTransport.close() states: "Close the transport. If the transport has a buffer for outgoing data, buffered data will be flushed asynchronously."

The _UnixWritePipeTransport._close method, which is called by close() contains this code in Python 3.5.1

    def _close(self, exc=None):
        self._closing = True
        if self._buffer:
            self._loop.remove_writer(self._fileno)
        self._buffer.clear()
        self._loop.remove_reader(self._fileno)
        self._loop.call_soon(self._call_connection_lost, exc)

In this context, _buffer is a list of bytes objects comprising yet-to-be-written data. Note that close() removes the writer if _buffer is *not* empty, so the buffered data will never be written.  I believe this conditional should be inverted, so the writer is removed only if the buffer is *empty*. So:

        if not self._buffer:
            self._loop.remove_writer(self._fileno)

Arguably though, a more robust and easier to read test would be to call get_write_buffer_size(), like this:

        if self.get_write_buffer_size() == 0:
            self._loop.remove_writer(self._fileno)

Note that in the event that the writer is not removed by _close(), the writer will
remove itself when the buffer does eventually become empty.

Assuming my analysis is correct, and a fix is made to close(), then abort() will also need to be modified to *not* flush buffered writes, as the documentation for abort() states "Close the transport immediately, without waiting for pending operations to complete. Buffered data will be lost."
History
Date User Action Args
2016-03-29 10:54:31Robert Smallshiresettype: behavior
2016-03-29 10:54:15Robert Smallshirecreate