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

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

Merged
merged 3 commits into from
Feb 17, 2024

Conversation

deedy5
Copy link
Contributor

@deedy5 deedy5 commented Feb 11, 2024

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
Copy link
Contributor Author

deedy5 commented Feb 12, 2024

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 deedy5 mentioned this pull request Feb 12, 2024
@deedy5
Copy link
Contributor Author

deedy5 commented Feb 15, 2024

@yifeikong

@perklet
Copy link
Collaborator

perklet commented Feb 17, 2024

Hi @deedy5, have you finished your PR?

@deedy5
Copy link
Contributor Author

deedy5 commented Feb 17, 2024

@yifeikong
yes

@perklet perklet merged commit 537d319 into lexiforest:main Feb 17, 2024
6 checks passed
@deedy5 deedy5 deleted the dev branch February 17, 2024 15:56
@deedy5 deedy5 changed the title Suppress Pending Task Messages During Shutdown Bugfix: async curl timer leak (Suppress Pending Task Messages During Shutdown) Feb 17, 2024
@deedy5
Copy link
Contributor Author

deedy5 commented Feb 21, 2024

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

@perklet
Copy link
Collaborator

perklet commented Feb 21, 2024

I'll be releasing 0.6 soon, probably tomorrow.

@perklet
Copy link
Collaborator

perklet commented Feb 22, 2024

0.6.0 was released.

@perklet
Copy link
Collaborator

perklet commented Feb 23, 2024

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.

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 this pull request may close these issues.

2 participants