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

Asyncio streams are not being closed #133

Closed
py-mine-bot opened this issue Feb 15, 2022 · 0 comments
Closed

Asyncio streams are not being closed #133

py-mine-bot opened this issue Feb 15, 2022 · 0 comments
Labels
Github Import This was auto-imported from upstream repository meta: Not Enough Information Further clarification is needed

Comments

@py-mine-bot
Copy link
Collaborator

thesadru Authored by thesadru
Jun 2, 2021
Closed Jun 16, 2021


Whenever an async function is called a connection gets opened, but this connection never gets closed. This causes exceptions to be raised

ERROR:asyncio: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

This can be fixed by simply calling connection.writer.close() right before the return.

@py-mine-bot
Copy link
Collaborator Author

kevinkjt2000 Authored by kevinkjt2000
Jun 15, 2021


@thesadru Would you mind verifying this is fixed with v6.1.0?

@py-mine-bot
Copy link
Collaborator Author

thesadru Authored by thesadru
Jun 15, 2021


The error persists, there's still no connection.close() in async_status.

@py-mine-bot
Copy link
Collaborator Author

thesadru Authored by thesadru
Jun 15, 2021


Actually, to add to this I think it'd be a good idea to just put the close() either in some finally block or in the connection's __del__ to make sure it gets closed even on errors.

@py-mine-bot
Copy link
Collaborator Author

kevinkjt2000 Authored by kevinkjt2000
Jun 15, 2021


If you have a script that produces the error at least semi-reliably, I would convert that into a unit test to fix this. Please share 😄

@py-mine-bot
Copy link
Collaborator Author

thesadru Authored by thesadru
Jun 16, 2021


I'm super sorry. It seems I caused the bug myself by messing with the library's source code. I didn't find out because the library was technically up to date. I reinstalled it and it works fine now. I apologize for the inconvenience.

@py-mine-bot
Copy link
Collaborator Author

thesadru Authored by thesadru
Jun 16, 2021


Oops, guess I was too hasty writing this off as just me messing with the source code. I'm getting the error again but only when I'm using discord.py and pinging specifically my friend's server. This is most likely a bug in discord.py rather than mcstatus. I couldn't figure out what's the reason but maybe you could.

import mcstatus
import asyncio
import discord

loop = asyncio.get_event_loop()
server = mcstatus.MinecraftServer.lookup('m.bylex.cz')
loop.create_task(server.async_status())

bot = discord.Client()
bot.run(TOKEN)
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

@py-mine-bot
Copy link
Collaborator Author

kevinkjt2000 Authored by kevinkjt2000
Jun 16, 2021


It is not yet clear to me which library would cause that asyncio stacktrace. I also could not reproduce the issue when I tried earlier. Would you mind making a small reproduce repo (or gist) that has dependencies locked to the same versions that are giving you the error? I'm picturing:

poetry new mcstatus-issue-133
cd mcstatus-issue-133
poetry add discord.py mcstatus
# add a main.py file with the script that can reproduce the error
# also, please use `os.environ` for the bot token, so you don't accidentally share it
poetry run python main.py  # keep running this until you can reproduce the issue
# add all that stuff to a gist or github repo and link to it in this discussion

Of course, feel free to deviate from these instructions so long as it is easy to reproduce the error. It's hard for me to fix if I can't make it happen and debug it.

@py-mine-bot
Copy link
Collaborator Author

@py-mine-bot
Copy link
Collaborator Author

kevinkjt2000 Authored by kevinkjt2000
Jun 16, 2021


This does seem to be a windows specific bug related to asyncio sockets using Protractor. I found a lot of good information in this issue and a fix encode/httpx#914 (comment)

This seems to make the issue disappear:

import asyncio
import os
import sys

import discord
import mcstatus

if sys.version_info[0] == 3 and sys.version_info[1] >= 8 and sys.platform.startswith('win'):
    asyncio.set_event_loop_policy(asyncio.WindowsSelectorEventLoopPolicy())

server = mcstatus.MinecraftServer.lookup('m.bylex.cz')

async def server_status():
    status = await server.async_status()
    print(status.raw)

loop = asyncio.get_event_loop()
task = loop.create_task(server_status())

bot = discord.Client()
bot.run(os.environ['TOKEN'])

I'm going to close the issue and mark it as invalid since it is not a bug within mcstatus as far as I can tell. I'll add this to my ongoing list of things to include in #136

@py-mine-bot
Copy link
Collaborator Author

thesadru Authored by thesadru
Jun 3, 2021


This bug would be fixed with #134

@py-mine-bot py-mine-bot added Github Import This was auto-imported from upstream repository meta: Not Enough Information Further clarification is needed labels Feb 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Github Import This was auto-imported from upstream repository meta: Not Enough Information Further clarification is needed
Projects
None yet
Development

No branches or pull requests

1 participant