Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Sample code causes Windows OSError #10038 #917

Closed
ahrbe1 opened this issue Jul 18, 2021 · 22 comments · Fixed by #918
Closed

Sample code causes Windows OSError #10038 #917

ahrbe1 opened this issue Jul 18, 2021 · 22 comments · Fixed by #918

Comments

@ahrbe1
Copy link

ahrbe1 commented Jul 18, 2021

Code from the README:

from zeroconf import ServiceBrowser, Zeroconf

class MyListener:

    def remove_service(self, zeroconf, type, name):
        print("Service %s removed" % (name,))

    def add_service(self, zeroconf, type, name):
        info = zeroconf.get_service_info(type, name)
        print("Service %s added, service info: %s" % (name, info))

zeroconf = Zeroconf()
listener = MyListener()
browser = ServiceBrowser(zeroconf, "_http._tcp.local.", listener)

try:
    input("Press enter to exit...\n\n")
finally:
    zeroconf.close()

Causes this error on Windows:

Exception in callback _ProactorBasePipeTransport._call_connection_lost(None)
handle: <Handle _ProactorBasePipeTransport._call_connection_lost(None)>
Traceback (most recent call last):
  File "C:\Program Files\Python39\lib\asyncio\events.py", line 80, in _run
    self._context.run(self._callback, *self._args)
  File "C:\Program Files\Python39\lib\asyncio\proactor_events.py", line 162, in _call_connection_lost
    self._sock.shutdown(socket.SHUT_RDWR)
OSError: [WinError 10038] An operation was attempted on something that is not a socket
Exception in callback _ProactorBasePipeTransport._call_connection_lost(None)
handle: <Handle _ProactorBasePipeTransport._call_connection_lost(None)>
Traceback (most recent call last):
  File "C:\Program Files\Python39\lib\asyncio\events.py", line 80, in _run
    self._context.run(self._callback, *self._args)
  File "C:\Program Files\Python39\lib\asyncio\proactor_events.py", line 162, in _call_connection_lost
    self._sock.shutdown(socket.SHUT_RDWR)
OSError: [WinError 10038] An operation was attempted on something that is not a socket
Exception in callback _ProactorBasePipeTransport._call_connection_lost(None)
handle: <Handle _ProactorBasePipeTransport._call_connection_lost(None)>
Traceback (most recent call last):
  File "C:\Program Files\Python39\lib\asyncio\events.py", line 80, in _run
    self._context.run(self._callback, *self._args)
  File "C:\Program Files\Python39\lib\asyncio\proactor_events.py", line 162, in _call_connection_lost
    self._sock.shutdown(socket.SHUT_RDWR)
OSError: [WinError 10038] An operation was attempted on something that is not a socket

On Python 3.9.3.

Is there a way to shut down this library cleanly?

@bdraco
Copy link
Member

bdraco commented Jul 18, 2021

Does the problem still happen if you limit to just the primary network interface?

@ahrbe1
Copy link
Author

ahrbe1 commented Jul 19, 2021

Limiting it to the primary interface seems to resolve the issue. It seems I'll need to figure out how to programmatically determine my primary network address (e.g. in the presence of virtual adapters from VirtualBox, etc)

@bdraco
Copy link
Member

bdraco commented Jul 19, 2021

We have quite a few checks in zeroconf/_utils/net.py to skip problematic interfaces. If you can figure out the root cause of the issue we can probably detect and auto skip the affected interface

@ahrbe1
Copy link
Author

ahrbe1 commented Jul 19, 2021

Is there a way to see what IP's it's using with InterfaceChoice.All?

The following snippet also seems to fix the issue, but I feel like there's probably a better way:

# try to determine 'real' network adapter by resolving Cloudflare's DNS
def get_ip_address():
    s = socket.socket(socket.AF_INET, socket.SOCK_DGRAM)
    s.connect(('1.1.1.1', 53))
    return s.getsockname()[0]

[...]

zeroconf = Zeroconf(interfaces=[get_ip_address()])

@ahrbe1
Copy link
Author

ahrbe1 commented Jul 19, 2021

Ok. Apparently I tested that wrong. It's still an issue. The difference was I was hitting CTRL-C vs ENTER.

If I hit CTRL-C, it exits silently. If I hit ENTER, it gives the OSErrors.

In the original snippet, I get 3x OSErrors because the InterfaceChoice.All yields 3 interfaces: the real one, a virtualbox one, and 127.0.0.1. If I manually specify just the real interface, well, I only get one error instead of 3 of them.

So something else is going on here.

@bdraco
Copy link
Member

bdraco commented Jul 19, 2021

https://github.com/jstasiak/python-zeroconf/blob/master/zeroconf/_utils/net.py#L73
https://github.com/jstasiak/python-zeroconf/blob/master/zeroconf/_utils/net.py#L77

If you do keyboard interrupt or end the program right away, the listeners won't be setup so they cannot shutdown cleanly because they are not fully setup when you terminate the program.

@bdraco
Copy link
Member

bdraco commented Jul 19, 2021

Its almost definitely https://bugs.python.org/issue43253

The PR I opened above should downgrade it to debug since it will likely need to be resolved in python itself. Please give it a shot and let me know if it prevents the backtrace.

@bdraco
Copy link
Member

bdraco commented Jul 19, 2021

Seems like it can happen anytime on windows and not just during startup?

@ahrbe1
Copy link
Author

ahrbe1 commented Jul 19, 2021

So the code snippet linked in that post gives the same error.

import socket
s1, s2 = socket.socketpair()
import asyncio
async def test():
    r1, w1 = await asyncio.open_connection(sock=s1)
    r2, w2 = await asyncio.open_connection(sock=s2)
    s1.close()
asyncio.run(test())
Exception in callback _ProactorBasePipeTransport._call_connection_lost(ConnectionAbo...e, 1236, None))
handle: <Handle _ProactorBasePipeTransport._call_connection_lost(ConnectionAbo...e, 1236, None))>
Traceback (most recent call last):
  File "C:\Program Files\Python39\lib\asyncio\events.py", line 80, in _run
    self._context.run(self._callback, *self._args)
  File "C:\Program Files\Python39\lib\asyncio\proactor_events.py", line 162, in _call_connection_lost
    self._sock.shutdown(socket.SHUT_RDWR)
OSError: [WinError 10038] An operation was attempted on something that is not a socket

I suppose I can side-step the error by not calling zeroconf.close().

@bdraco
Copy link
Member

bdraco commented Jul 19, 2021

Can you try the linked pr? #918

@ahrbe1
Copy link
Author

ahrbe1 commented Jul 19, 2021

Ok. Just did. Yeah, that seems to silence the error.

@ahrbe1
Copy link
Author

ahrbe1 commented Jul 19, 2021

Ok. Nevermind. I had some working changes in my copy of zeroconf. Blew that away and reinstalled with that PR. Still has that error.

@bdraco
Copy link
Member

bdraco commented Jul 19, 2021

I was hoping we could close it in connection_lost, but apparently that is prohibited

  /usr/lib/python3.8/asyncio/trsock.py:20: DeprecationWarning: Using close() method on sockets returned from get_extra_info('socket') will be prohibited in asyncio 3.9. Please report your use case to bugs.python.org.

@ahrbe1
Copy link
Author

ahrbe1 commented Jul 19, 2021

I hesitate to even mention this, but in proactor_events.py in _call_connection_lost(), they shutdown the socket without checking if it's already been closed. I think that's the issue. Somewhere this thing is getting closed before we get to this code.

    def _call_connection_lost(self, exc):
        try:
            self._protocol.connection_lost(exc)
        finally:
            # XXX If there is a pending overlapped read on the other
            # end then it may fail with ERROR_NETNAME_DELETED if we
            # just close our end.  First calling shutdown() seems to
            # cure it, but maybe using DisconnectEx() would be better.
            if hasattr(self._sock, 'shutdown'):
                self._sock.shutdown(socket.SHUT_RDWR)

I think this should rather be something like:

    def _call_connection_lost(self, exc):
        try:
            self._protocol.connection_lost(exc)
        finally:
            # XXX If there is a pending overlapped read on the other
            # end then it may fail with ERROR_NETNAME_DELETED if we
            # just close our end.  First calling shutdown() seems to
            # cure it, but maybe using DisconnectEx() would be better.
            if hasattr(self._sock, 'shutdown'):
                if not self._sock._closed:
                    self._sock.shutdown(socket.SHUT_RDWR)

There's unfortunately no public API to check if a socket is closed, that I can tell.

@bdraco
Copy link
Member

bdraco commented Jul 19, 2021

Protractor is closing it on connection_lost here

https://github.com/python/cpython/blob/3.9/Lib/asyncio/proactor_events.py#L163

Selector is closing it on connection_lost as well

https://github.com/python/cpython/blob/3.9/Lib/asyncio/selector_events.py#L738

@ahrbe1
Copy link
Author

ahrbe1 commented Jul 19, 2021

Well, interestingly the code snippet from python issue 43253 no longer gives an error with that change.

@bdraco
Copy link
Member

bdraco commented Jul 19, 2021

Can you give the latest version of #918 a try. I verified on linux that the underlying socket still gets closed.

@ahrbe1
Copy link
Author

ahrbe1 commented Jul 19, 2021

Ok. I think that resolves it.

I made sure to revert my patch to python, pull the updated PR, and wipe my existing zeroconf install. Ran the demo script a bunch of times, and didn't see any errors.

@bdraco
Copy link
Member

bdraco commented Jul 19, 2021

Thanks. I'll do testing on Mac tomorrow and push a new release if everything looks ok

@ahrbe1
Copy link
Author

ahrbe1 commented Jul 19, 2021

Thanks! Sounds good!

bdraco added a commit that referenced this issue Jul 19, 2021
- The socket was closed during shutdown before asyncio's connection_lost
  handler had a chance to close it which resulted in a traceback on
  win32.

- Fixes #917
@bdraco
Copy link
Member

bdraco commented Jul 19, 2021

Mac testing looks good. I'll try to get the release out before sleep.

@bdraco
Copy link
Member

bdraco commented Jul 19, 2021

0.33.0 published. enjoy!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants