lexiforest / curl_cffi

Python binding for curl-impersonate fork via cffi. A http client that can impersonate browser tls/ja3/http2 fingerprints.
https://curl-cffi.readthedocs.io/
MIT License
2.54k stars 269 forks source link

Bugfix: async curl timer leak (Suppress Pending Task Messages During Shutdown) #245

Closed deedy5 closed 9 months ago

deedy5 commented 9 months ago

Reason:

Sometimes there are messages like this one:

Task was destroyed but it is pending!
task: <Task cancelling name='Task-2' coro=<AsyncCurl._force_timeout() done, defined at .venv/lib/python3.12/site-packages/curl_cffi/aio.py:164> wait_for=<Future cancelled>>

Description:

This PR addresses the issue of warning messages being logged when the AsyncCurl object is closed. Specifically, it resolves the "Task was destroyed but it is pending!" warnings that occur when the _checker task is cancelled.

Changes Made:

Modified close method: added a try/except block to catch and ignore asyncio.CancelledError when cancelling the _checker task. This prevents the warning message from being logged.

def close(self):
    # Cancel the force timeout checker
    self._checker.cancel()
    # Wait for the force timeout checker to finish
    try:
        self.loop.run_until_complete(self._checker)
    except asyncio.CancelledError:
        # This is expected since we just cancelled the task
        pass
    # ... rest of the cleanup code ...

These changes ensure that the _checker task is cancelled gracefully, and the cancellation is confirmed before continuing with the cleanup process. This should eliminate the warning messages related to pending tasks during object closure.

deedy5 commented 9 months ago

Problem with checks:

Build, test and release / Build bdist wheels and test (ubuntu-22.04) Build, test and release / Build bdist wheels and test (macos-12) Build, test and release / Build bdist wheels and test (macos-14) Build, test and release / Build bdist wheels and test (windows-2019)

These checks cannot be completed and are canceled after 6 hours.

deedy5 commented 9 months ago

@yifeikong

perklet commented 9 months ago

Hi @deedy5, have you finished your PR?

deedy5 commented 9 months ago

@yifeikong yes

deedy5 commented 9 months ago

@yifeikong Hi. Can you release a new beta version with this change?

perklet commented 9 months ago

I'll be releasing 0.6 soon, probably tomorrow.

perklet commented 9 months ago

0.6.0 was released.

perklet commented 9 months ago

The nest_asyncio trick will also need to be applied to other people's code 😢, not just the tests, otherwise it will raise errors, see #254. Thus I changed AsyncSession.close to a coroutine, which seems to be the common way.

A new release(0.6.1) was published to address this issue, you may need to adjust your code accordingly.