Dinnerbone / mcstatus

A Python class for checking the status of an enabled Minecraft server
http://dinnerbone.com/minecraft/tools/status/
1.11k stars 146 forks source link

Asyncio streams are not being closed #133

Closed thesadru closed 3 years ago

thesadru commented 3 years ago

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.

thesadru commented 3 years ago

This bug would be fixed with #134

kevinkjt2000 commented 3 years ago

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

thesadru commented 3 years ago

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

thesadru commented 3 years ago

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.

kevinkjt2000 commented 3 years ago

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 😄

thesadru commented 3 years ago

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.

thesadru commented 3 years ago

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
kevinkjt2000 commented 3 years ago

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.

thesadru commented 3 years ago

https://gist.github.com/thesadru/131af37792b300c002d09de4ec3b0a44

kevinkjt2000 commented 3 years ago

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 https://github.com/encode/httpx/issues/914#issuecomment-622586610

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