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

Circular reference with RequestHandler.on_connection_close #95

Closed
elephantum opened this issue May 25, 2010 · 6 comments
Closed

Circular reference with RequestHandler.on_connection_close #95

elephantum opened this issue May 25, 2010 · 6 comments

Comments

@elephantum
Copy link

The following circular reference can be observed in runtime:

RequestHandler -> .request -> HTTPRequest -> .connection -> HTTPConnection -> .stream -> IOStream -> ._close_callback -> RequestHandler.on_connection_close -> RequestHandler

This prevents gc to collect RequestHandler and all the associated objects, which leads to evergrowing memory consumption.

There is very simple fix for this problem: http://github.com/elephantum/tornado/commit/8ffbe0e64df2c36b93e8ca66abfe483b211efd52

@bdarnell
Copy link
Member

It's a cycle all right, but without some external reference the whole lot should be garbage collected at once. Is any of these objects being referred to from outside the cycle?

@elephantum
Copy link
Author

It seems that python's cyclic garbage collector has some latency. i.e. objects are not collected at once, but after some time. This means larger memory consumption for unnecessary objects.

For example here is picture from monitoring of my project in production environment before and after the proposed patch.

http://skitch.com/elephantum/dff2m/monik

@elephantum
Copy link
Author

And one more point: python's cyclic gc doesn't work on objects which has del method defined. It means, that if any of users define del method in RequestHandler subclass then memory would never be freed.

@bdarnell
Copy link
Member

Thanks, I've committed the fix. (locally; I'll push it to github once I've had a chance to do a little testing).

@elephantum
Copy link
Author

thanks.

BTW. You've mentioned testing more than once in issues and mailing list. I can't find any testing code in tornado/ though. Does it mean that you have test code somewhere outside github?

@bdarnell
Copy link
Member

I don't have test code for tornado itself. I have unittests for an app that uses tornado that exercise some basic functionality, but when I talk about testing I mostly mean running my app with a new version of tornado and poking around in a browser.

This issue was closed.
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

No branches or pull requests

2 participants