dabeaz / curio

Good Curio!
Other
4.01k stars 240 forks source link

open_connection failed when called with source_addr. #291

Closed guyingbo closed 5 years ago

guyingbo commented 5 years ago

version: 0.9 curio/network.py

async def open_connection(host, port, *, ssl=None, source_addr=None, server_hostname=None,
                          alpn_protocols=None):
    '''
    Create a TCP connection to a given Internet host and port with optional SSL applied to it.
    '''
    if server_hostname and not ssl:
        raise ValueError('server_hostname is only applicable with SSL')

    sock = await socket.create_connection((host, port), source_addr)

    try:
        # Apply SSL wrapping to the connection, if applicable
        if ssl:
            sock = await _wrap_ssl_client(sock, ssl, server_hostname, alpn_protocols)

        return sock
    except Exception:
        sock._socket.close()
        raise

failed when source_addr is provided. solution:

async def open_connection(host, port, *, ssl=None, source_addr=None, server_hostname=None,
                          alpn_protocols=None):
    '''
    Create a TCP connection to a given Internet host and port with optional SSL applied to it.
    '''
    if server_hostname and not ssl:
        raise ValueError('server_hostname is only applicable with SSL')

    import socket as so
    sock = await socket.create_connection((host, port), so._GLOBAL_DEFAULT_TIMEOUT, source_addr)

    try:
        # Apply SSL wrapping to the connection, if applicable
        if ssl:
            sock = await _wrap_ssl_client(sock, ssl, server_hostname, alpn_protocols)

        return sock
    except Exception:
        sock._socket.close()
        raise

Add so._GLOBAL_DEFAULT_TIMEOUT makes it works. It would be a better way if I can write code like this

sock = await socket.create_connection((host, port), source_address=source_addr)

however, run_in_thread can't take kwargs arguments because it is defined as this:

async def run_in_thread(callable, *args, call_on_cancel=None):
dabeaz commented 5 years ago

Quick comment: run_in_thread() can take kwargs if you wrap it with partial() or a lambda. For example:

from functools import partial
await run_in_thread(partial(callable, keyword=value))

This is actually the recommended way to pass keyword arguments to all of Curio's functions that launch tasks, functions, and other similar things.

Can you say a bit more about this so._GLOBAL_DEFAULT_TIMEOUT workaround. I'm not really sure I understand the rest of this bug report on quick read.

guyingbo commented 5 years ago

Because python's socket.create_connection is defined as:

def create_connection(address, timeout=_GLOBAL_DEFAULT_TIMEOUT,
                      source_address=None):

and curio's socket.create_connection is defined as:

@wraps(_socket.create_connection)
async def create_connection(*args, **kwargs):
    sock = await workers.run_in_thread(_socket.create_connection, *args, **kwargs)
    return io.Socket(sock)

, in curio, we should call

sock = await socket.create_connection((host, port), timeout, source_addr)

instead of

sock = await socket.create_connection((host, port), source_addr)

which misses the timeout parameter.

dabeaz commented 5 years ago

Ah. Got it. Will make a patch for this.

dabeaz commented 5 years ago

Fixed.