Docs/behavior mismatch for reuse_address default

[b3b]

Hi, thanks for maintaining this great library.

I noticed a docs/behavior mismatch around reuse_address.
Before 2.9.0, reuse_address defaulted to None, so omitting it preserved asyncio default behavior.
However, the default value was later changed to False, so reuse is now explicitly disabled unless requested.

This seems to have happened when type annotations were added: https://github.com/ronf/asyncssh/compare/v2.8.1...v2.9.0.patch

And the docs still say:

If not specified, this will be automatically set to True on UNIX.

Since this behavior has existed for a while, maybe this should be treated as a docs issue rather than a regression.

[ronf]

Thanks very much for reporting this! It was definitely a mistake to switch from None to False here. The intention has always been to mirror the behavior of asyncio's create_server() method exactly, passing reuse_address and reuse_port through and letting the Python library decide what the behavior is when these arguments are not specified.

While it has been a while since this change was made, I think it still makes sense to fix this, particularly given that the comment about this has been in place (copied from asyncio.create_server()) the entire time. Anyone setting these values explicitly to True or False will see no behavior difference. The only difference is that reuse_address will be enabled by default on UNIX if it is not explicitly set to False, and I think that will be a good thing.

Here's the fix I'm planning to check in:

diff --git a/asyncssh/connection.py b/asyncssh/connection.py
index 89fdb16..8188b2f 100644
--- a/asyncssh/connection.py
+++ b/asyncssh/connection.py
@@ -553,7 +553,7 @@ async def _connect(options: _Options, config: DefTuple[ConfigPaths],
 async def _listen(options: _Options, config: DefTuple[ConfigPaths],
                   loop: asyncio.AbstractEventLoop, flags: int,
                   backlog: int, sock: Optional[socket.socket],
-                  reuse_address: bool, reuse_port: bool,
+                  reuse_address: Optional[bool], reuse_port: Optional[bool],
                   conn_factory: Callable[[], _Conn],
                   msg: str) -> 'SSHAcceptor':
     """Make inbound TCP or SSH tunneled listener"""
@@ -9353,7 +9353,8 @@ async def listen(host = '', port: DefTuple[int] = (), *,
                  tunnel: DefTuple[_TunnelListener] = (),
                  family: DefTuple[int] = (), flags:int = socket.AI_PASSIVE,
                  backlog: int = 100, sock: Optional[socket.socket] = None,
-                 reuse_address: bool = False, reuse_port: bool = False,
+                 reuse_address: Optional[bool] = None,
+                 reuse_port: Optional[bool] = None,
                  acceptor: _AcceptHandler = None,
                  error_handler: _ErrorHandler = None,
                  config: DefTuple[ConfigPaths] = (),
@@ -9411,7 +9412,7 @@ async def listen(host = '', port: DefTuple[int] = (), *,
            port other existing sockets are bound to, so long as they all
            set this flag when being created. If not specified, the
            default is to not allow this. This option is not supported
-           on Windows or Python versions prior to 3.4.4.
+           on Windows.
        :param acceptor: (optional)
            A `callable` or coroutine which will be called when the
            SSH handshake completes on an accepted connection, taking
@@ -9441,8 +9442,8 @@ async def listen(host = '', port: DefTuple[int] = (), *,
        :type flags: flags to pass to :meth:`getaddrinfo() <socket.getaddrinfo>`
        :type backlog: `int`
        :type sock: :class:`socket.socket` or `None`
-       :type reuse_address: `bool`
-       :type reuse_port: `bool`
+       :type reuse_address: `bool` or `None`
+       :type reuse_port: `bool` or `None`
        :type acceptor: `callable` or coroutine
        :type error_handler: `callable`
        :type config: `list` of `str`
@@ -9478,7 +9479,8 @@ async def listen_reverse(host = '', port: DefTuple[int] = (), *,
                          family: DefTuple[int] = (),
                          flags: int = socket.AI_PASSIVE, backlog: int = 100,
                          sock: Optional[socket.socket] = None,
-                         reuse_address: bool = False, reuse_port: bool = False,
+                         reuse_address: Optional[bool] = None,
+                         reuse_port: Optional[bool] = None,
                          acceptor: _AcceptHandler = None,
                          error_handler: _ErrorHandler = None,
                          config: DefTuple[ConfigPaths] = (),
@@ -9545,7 +9547,7 @@ async def listen_reverse(host = '', port: DefTuple[int] = (), *,
            port other existing sockets are bound to, so long as they all
            set this flag when being created. If not specified, the
            default is to not allow this. This option is not supported
-           on Windows or Python versions prior to 3.4.4.
+           on Windows.
        :param acceptor: (optional)
            A `callable` or coroutine which will be called when the
            SSH handshake completes on an accepted connection, taking
@@ -9580,8 +9582,8 @@ async def listen_reverse(host = '', port: DefTuple[int] = (), *,
        :type flags: flags to pass to :meth:`getaddrinfo() <socket.getaddrinfo>`
        :type backlog: `int`
        :type sock: :class:`socket.socket` or `None`
-       :type reuse_address: `bool`
-       :type reuse_port: `bool`
+       :type reuse_address: `bool` or `None`
+       :type reuse_port: `bool` or `None`
        :type acceptor: `callable` or coroutine
        :type error_handler: `callable`
        :type config: `list` of `str`

[b3b]

Great, these defaults make sense to me. I hit this on Linux when omitting reuse_address.

[ronf]

Thanks - this is now available in commit 8ca6702 in the "develop" branch and will be included in the next release.