asyncio icon indicating copy to clipboard operation
asyncio copied to clipboard

Error on closing transport right after connection was made.

Open fr0stbite opened this issue 9 years ago • 16 comments

Hello,

I was experimenting with asyncio a lot lately and I noticed that it provides very small support for writing asynchronnous UDP applications. I wanted to write a very simple module, that would allow me to open a UDP socket, send some data over it and close it immediately. I tried to accomplish this by implementing my own DatagramProtocol.

class SendProtocol:

    def connection_made(self, transport):
        transport.sendto(data)
        transport.close()

However, this implementation produces an error everytime the transport.close method is called, because the _read_ready action fails.

I was trying to find something that would help me in the docs, and I came across asyncio.streams.open_connection method, that returns StreamReader and StreamWriter objects, which I could use to control the flow of my application. Unfortunately, this method is only compatible with TCP, not UDP. I would like to ask if there's an equivalent of this method for UDP transport that I overlooked. Or if you plan to add it in the future.

Cheers!

fr0stbite avatar Jul 01 '16 09:07 fr0stbite

Can you show the whole (minimal) program, and the specific error you got? Do you think the error is due to calling transport.close() inside your connection_made()?

A streams wrapper around UDP would set entirely wrong expectations, since streams assume reliable byte streams where the boundaries between packages are irrelevant (like TCP), but UDP provides lossy datagrams.

gvanrossum avatar Jul 01 '16 16:07 gvanrossum

I've written some simple examples to demonstrate the issue. You can read the source code below.

# client.py

import asyncio

class SendProtocol:
    def __init__(self, message, loop):
        self.message = message
        self.loop = loop

    def connection_made(self, transport):
        print("Sending:", self.message)
        transport.sendto(self.message.encode())
        transport.close()

    def datagram_received(self, data, addr):
        print("Received:", data.decode())

    def error_received(self, exception):
        print("Error occurred:", exception)

    def connection_lost(self, exception):
        print("Socket closed.")
        self.loop.stop()

loop = asyncio.get_event_loop()
address = ("localhost", 9999)
message = "Hello World!"

connect = loop.create_datagram_endpoint(
    lambda: SendProtocol(message, loop),
    remote_addr=address
)
transport, protocol = loop.run_until_complete(connect)

loop.run_forever()
loop.close()

# server.py

import asyncio

class DummyServerProtocol:

    def connection_made(self, transport):
        pass

    def datagram_received(self, data, address):
        message = data.decode()
        print("Received {} from {}".format(message, address))

    def error_received(self, exception):
        print("Error occurred:", exception)

    def connection_lost(self, exception):
        print("Socket closed.")
        self.loop.stop()

address = ("localhost", 9999)

loop = asyncio.get_event_loop()

print("Listening for datagrams at {}:{}.".format(*address))

listen = loop.create_datagram_endpoint(
    DummyServerProtocol, local_addr=address
)
transport, protocol = loop.run_until_complete(listen)

try:
    loop.run_forever()
except KeyboardInterrupt:
    pass

transport.close()
loop.close()

print("Server stopped.")

When I run this on Linux (Ubuntu 16.04 64-bit, Python 3.5.1), everything seems to work just fine. However, when I run the same code on Windows (Win7 64-bit, Python 3.5.1) it throws following.

Traceback (most recent call last):
  File "C:\Users\fr0stbite\Documents\GitProjects\asyncio\udp\client.py", line 35, in <module>
    loop.run_forever()
  File "C:\Python35-32\lib\asyncio\base_events.py", line 295, in run_forever
    self._run_once()
  File "C:\Python35-32\lib\asyncio\base_events.py", line 1218, in _run_once
    event_list = self._selector.select(timeout)
  File "C:\Python35-32\lib\selectors.py", line 314, in select
    r, w, _ = self._select(self._readers, self._writers, [], timeout)
  File "C:\Python35-32\lib\selectors.py", line 305, in _select
    r, w, x = select.select(r, w, w, timeout)
OSError: [WinError 10038] An operation was attempted on something that is not a socket

So this seems to be Windows-specific issue. Do you have any ideas on how to solve this problem? I will try to go through asyncio's sources and see if I can provide some more feedback.

fr0stbite avatar Jul 01 '16 18:07 fr0stbite

Yeah, that's a Windows-specific error message. I don't have access to a Windows box where I can try this, and I don't see an obvious bug in your code. So there could be a bug in asyncio for Windows, or there could be a non-obvious, subtle bug in your code, or there could be something in your configuration or environment...

Are you sure you're using 64-bit? The path "Python35-32" suggests that Python is built for 32 bits (though that's probably a red herring -- Windows itself may well be capable of using 64 bits).

You might also try if Python 3.5.2 improves things -- though it could also make things worse, so be prepared to uninstall and go back to 3.5.1 if you try that.

Sorry I can't be more helpful!

gvanrossum avatar Jul 02 '16 05:07 gvanrossum

I was probably using wrong Python build after all. But even after switching to Python 3.5.2 for 64-bit machines, the problem persists.

fr0stbite avatar Jul 02 '16 06:07 fr0stbite

To dig into this more, print the file descriptors in the select.

On Friday, July 1, 2016, Tomáš Rottenberg [email protected] wrote:

I was probably using wrong Python build after all. But even after switching to Python 3.5.2 for 64-bit machines, the problem persists.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/python/asyncio/issues/367#issuecomment-230087470, or mute the thread https://github.com/notifications/unsubscribe/ACwrMgo5kCgS1Pyo_8SivTBoj7dnGjkSks5qRgn_gaJpZM4JC_pc .

--Guido (mobile)

gvanrossum avatar Jul 02 '16 14:07 gvanrossum

Thanks for the tip! I will try to do as much as I can to resolve this and see what I can do.

fr0stbite avatar Jul 03 '16 13:07 fr0stbite

I have done some tests and this is the best solution I have found so far.

class _SelectorDatagramTransport(_SelectorTransport):

    _buffer_factory = collections.deque

    def __init__(self, loop, sock, protocol, address=None,
                 waiter=None, extra=None):
        super().__init__(loop, sock, protocol, extra)
        self._address = address
        self._loop.call_soon(self._protocol.connection_made, self)
        # only start reading when connection_made() has been called
        self._loop.call_soon(self._test)
        if waiter is not None:
            # only wake up the waiter when connection_made() has been called
            self._loop.call_soon(futures._set_result_unless_cancelled,
                                 waiter, None)

    def _test(self):
        print("Closing:", self._closing)
        if not self._closing:
            # only add reader, if the connection was not requested to be closed
            self._loop.add_reader(self._sock_fd, self._read_ready)

Instead of calling self._loop.call_soon(self._loop.add_reader, self._sock_fd, self._read_ready) directly in the constructor, I have wrapped the call in a method called _test. Since the _test method itself is scheduled to be called only after the connection_made method is done executing, we can reliably check whether the transport was requested to be closed and if that's the case, prevent unnecessary registration of readers, which might cause errors.

fr0stbite avatar Jul 03 '16 16:07 fr0stbite

I have changed the title of this issue to make it more descriptive.

fr0stbite avatar Jul 22 '16 07:07 fr0stbite

@fr0stbite I can take a look at this tonight if the problem persists.

sethmlarson avatar Sep 13 '16 13:09 sethmlarson

Hey @SethMichaelLarson, that would be great!

The problem still persists. When I run the example code I have posted before it spits following output on the client's side.

Sending: Hello World!
Socket closed.
Traceback (most recent call last):
  File "C:\Users\fr0stbite\Projects\client.py", line 35, in <module>
    loop.run_forever()
  File "C:\Python352\lib\asyncio\base_events.py", line 345, in run_forever
    self._run_once()
  File "C:\Python352\lib\asyncio\base_events.py", line 1276, in _run_once
    event_list = self._selector.select(timeout)
  File "C:\Python352\lib\selectors.py", line 323, in select
    r, w, _ = self._select(self._readers, self._writers, [], timeout)
  File "C:\Python352\lib\selectors.py", line 314, in _select
    r, w, x = select.select(r, w, w, timeout)
OSError: [WinError 10038] An operation was attempted on something that is not a socket

The server, however, does not complain about anything.

I have tested this on Windows 7 and Windows 10 with 64 bit version of Python 3.5.2.

If you need any more feedback, please let me know!

fr0stbite avatar Sep 13 '16 15:09 fr0stbite

@fr0stbite I'll be testing on Windows 10 Python 3.5.2 64 bit so I'll be able to reproduce easily. Hopefully we can get to the bottom of this.

sethmlarson avatar Sep 13 '16 15:09 sethmlarson

Awesome! Please keep us updated on your progress.

fr0stbite avatar Sep 13 '16 15:09 fr0stbite

@fr0stbite It seems the problem is not with Windows per-say and more that Windows only has one Selector implementation available: SelectSelector. On Linux there are many other Selectors that are chosen as the default because they are faster. Working on making SelectSelector better at handling stale file descriptors.

sethmlarson avatar Sep 16 '16 02:09 sethmlarson

@fr0stbite Hey, if you have time could you check to see if this PR fixes your problem on your side? (Windows 7) as I currently only have a Windows 10 machine.

sethmlarson avatar Sep 16 '16 14:09 sethmlarson

@SethMichaelLarson Although I cannot test out your fix on Windows 7 machine, since I have updated the one I used before to Windows 10, applying your changes on my Windows 10 computer seems to fix the issue.

fr0stbite avatar Sep 16 '16 18:09 fr0stbite

@fr0stbite Thanks for confirming!

sethmlarson avatar Sep 16 '16 18:09 sethmlarson