Gregory P. Smith <g...@krypto.org> added the comment:
[pasting in my investigation that I replied with on a mailing list:] The possible problem does appear real but it likely not frequently observed and is something most people reading the code wouldn't see as it's rather esoteric: https://blog.netherlabs.nl/articles/2009/01/18/the-ultimate-so_linger-page-or-why-is-my-tcp-not-reliable via https://stackoverflow.com/questions/8874021/close-socket-directly-after-send-unsafe describes the scenario. So does this apply in Python? Apparently. Our socket module close() is simply calling close as expected. https://github.com/python/cpython/blob/main/Modules/socketmodule.c#L3132 -> https://github.com/python/cpython/blob/main/Modules/socketmodule.c#L443 Our ftplib code that implicitly calls close on the data connection when exiting the ` with self.transfercmd(cmd, rest) as conn: ` context manager https://github.com/python/cpython/blob/main/Lib/ftplib.py#L498` Triggers a close() without a prior shutdown(socket.SHUT_RDWR) + read() -> EOF or timeout dance. In most protocols this isn't a big deal. ftp being so old as to rely solely on the TCP connection itself is the outlier. How often does this happen in practice? I don't know. I haven't heard of it happening, but how many people honestly use ftplib to send data between operating systems where a socket close triggering a TCP RST rather than FIN on the sender might be more likely in the past 20 years? I suspect not many. The code can be improved to prevent it. But I don't believe it'll be feasible to construct a real world not-mock-filled regression test that would fail without it. Potential solution: Do the shutdown+recv EOF dance as the final step inside of the storbinary and storlines `with self.transfercmd as conn` blocks. Technically speaking socket.socket.shutdown() is conditionally compiled but I don't know if any platforms lacking the socket shutdown API exist anymore (my guess is that conditional compilation for shutdown is a leftover from the 1990s). If so, a "if hasattr(conn, 'shutdown'):" conditional before the added logic would suffice. Looking at various ftp client source code (netkit ftp & netbsd ftp - both bsd derivatives) as well as a modern gonuts golang one I do not see them explicitly doing the shutdown dance. (I didn't dive in to figure out if it is hidden within their flclose() or conn.Close() implementations as that'd be surprising) ---------- nosy: +gregory.p.smith _______________________________________ Python tracker <rep...@bugs.python.org> <https://bugs.python.org/issue10202> _______________________________________ _______________________________________________ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
