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

Rescheduling timers doesn't work with asyncioreactor #9780

Closed
twisted-trac opened this issue Mar 19, 2020 · 2 comments
Closed

Rescheduling timers doesn't work with asyncioreactor #9780

twisted-trac opened this issue Mar 19, 2020 · 2 comments

Comments

@twisted-trac
Copy link

IlyaSkriblovsky's avatar @IlyaSkriblovsky reported
Trac ID trac#9780
Type defect
Created 2020-03-19 20:04:00Z

AsyncioSelectorReactor.callLater() returns DelayedCalls whose reset() method doesn't work as expected.

  1. reset() to later time simply doesn't work: timer gets fired at original time

  2. reset() to earlier works, but original internal asyncio's timer isn't cancelled and this causes an exception in AsyncioSelectorReactor's internal code

  3. This is not neccessarily a bug, but worth to be fixed too: current implementation of AsyncioSelectorReactor.callLater() leaves many cyclically linked objects after timer is fired. For the app that uses timers heavily this leads to significally higher memory usage (and cpu too due to much more often GC invocations)

Here the gist with runnable demos of all three issues:
https://gist.github.com/IlyaSkriblovsky/58876c9f97c617875e7b739fd0447807

Also attaching graph of ram usage % on my servers that I've eventually tried to run on asyncioreactor instead of epollreactor, but was forced to rollback due to unusual memory usage.

Attachments:

  • 2020-03-19_22-51.png (21486 bytes) - added by IlyaSkriblovsky on 2020-03-19 20:05:07Z - RAM Usage % graph that encouraged me to investigate the issue
Searchable metadata
trac-id__9780 9780
type__defect defect
reporter__IlyaSkriblovsky IlyaSkriblovsky
priority__normal normal
milestone__None None
branch__ 
branch_author__ 
status__closed closed
resolution__fixed fixed
component__core core
keywords__asyncio__callLater__review asyncio, callLater, review
time__1584648240812878 1584648240812878
changetime__1587596005321646 1587596005321646
version__None None
owner__Wim_Lewis__wiml_____ Wim Lewis <wiml@...>

@twisted-trac
Copy link
Author

IlyaSkriblovsky's avatar @IlyaSkriblovsky commented

Link to PR: #1232

@twisted-trac
Copy link
Author

wiml's avatar @wiml set owner to @wiml
@wiml set status to closed

In changeset bc7fef3

#!CommitTicketReference repository="" revision="bc7fef3ed53d1b839089bb4d1fca8ff25ab76266"
Fix AsyncioSelectorReactor.callLater().reset() and fix memory leaks

Author: IlyaSkriblovsky
Reviewer: wiml, roderigc
Fixes: ticket:9780

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant