mosquito / aiormq

Pure python AMQP 0.9.1 asynchronous client library
Other
268 stars 58 forks source link

asyncio.gather() on None #76

Open wbenny opened 4 years ago

wbenny commented 4 years ago

Here https://github.com/mosquito/aiormq/blob/master/aiormq/connection.py#L439 ... is this snippet of code:

        self._reader_task = None                                            # 1

        await asyncio.gather(
            self.__close_writer(writer), return_exceptions=True
        )

        await asyncio.gather(self._reader_task, return_exceptions=True)     # 2

self._reader_task is being set to None and then given to the asyncio.gather(). This obviously raises an exception:

  File "..\venv\lib\site-packages\aiormq\connection.py", line 445, in _on_close
    await asyncio.gather(self._reader_task, return_exceptions=True)
  File "..\lib\asyncio\tasks.py", line 806, in gather
    fut = ensure_future(arg, loop=loop)
  File "..\lib\asyncio\tasks.py", line 673, in ensure_future
    raise TypeError('An asyncio.Future, a coroutine or an awaitable is '
TypeError: An asyncio.Future, a coroutine or an awaitable is required

... which is usually conviniently and silently suppressed here: https://github.com/mosquito/aiormq/blob/master/aiormq/base.py#L139

        with suppress(Exception):
            await self._on_close(exc)
wbenny commented 4 years ago

I believe this line doesn't belong there at all:

await asyncio.gather(self._reader_task, return_exceptions=True)

It was introduced here: https://github.com/mosquito/aiormq/commit/21860d5dead20ad5b8202dde35a676d5e45d2860#diff-ad998bcf1174a988f9e7d57468f6f146R452

Is my understanding correct?

wbenny commented 4 years ago

Okay, I looked closer at the diff and realized that lined does belong there, but it should be executed before the self._reader_task = None.