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

memory leak with 0.12.0 #220

Closed
amir20 opened this issue Jan 25, 2019 · 17 comments
Closed

memory leak with 0.12.0 #220

amir20 opened this issue Jan 25, 2019 · 17 comments
Labels

Comments

@amir20
Copy link

amir20 commented Jan 25, 2019

  • uvloop version: 0.12.0
  • Python version: 3.7.2
  • Platform: Mac os and Docker
  • Can you reproduce the bug with PYTHONASYNCIODEBUG in env?: Have not tried yet

Recently I upgraded from 0.11.3 to 0.12.0. I use uvloop to constantly pull data from an API and insert into a database. After running for a couple of minutes, I found that my memory usage jumped from ~100MB to ~450mb. Has anybody else run into this issue? I have a lot going on so reproducing it with a smaller gist would be hard right now. But reverting back to 0.11.3 immediately fixed the issue. So I am guessing something is related to the upgrade.

@1st1
Copy link
Member

1st1 commented Jan 25, 2019

Wow, this is unfortunate.

Does your application use SSL connections?

@amir20
Copy link
Author

amir20 commented Jan 25, 2019

Yes it does.

@1st1
Copy link
Member

1st1 commented Jan 25, 2019

UDP? 0.12.0 has a couple of UDP-related changes. If you use UDP please try to run 0.12.0rc1 too.

The 0.12 release has a brand new SSL implementation, so the leak might be related to that. Ideally we need a script to reproduce it though.

@amir20
Copy link
Author

amir20 commented Jan 25, 2019

I am not using UDP. It is only HTTPS and called about 51 requests per second. I'll see what I can do to reproduce it. I have an API, mongo storage and a bunch of other things that need to be stripped.

I might just create a tight loop with the two versions and record the memory.

@1st1 1st1 added the bug label Jan 25, 2019
@amir20
Copy link
Author

amir20 commented Jan 26, 2019

Alright. I was able to reproduce this pretty easily.

Here is my script.

import asyncio
import os

import aiohttp
import uvloop
from async_timeout import timeout

asyncio.set_event_loop_policy(uvloop.EventLoopPolicy())


def headers():
    return dict(authorization=f"Bearer {os.getenv('API_TOKEN')}")


async def fetch(url):
    async with aiohttp.ClientSession(headers=headers()) as session:
        async with timeout(8):
            async with session.get(url) as response:
                data = await response.json()
                return response.status, data


for i in range(1000):
    future = fetch("https://api.clashofclans.com/v1/clans/%23UGJPVJR")
    code, response = asyncio.get_event_loop().run_until_complete(future)
    print(i, code)

I ran this with 0.11.x and after 200 iterations my memory was around 25MB. I used Docker so here is a screenshot.

screen shot 2019-01-26 at 2 07 09 pm

I did pip install -U uvloop and docker-compose build with the latest uvloop and after 200 iterations the memory is around 64MB.

Screenshot attached.

screen shot 2019-01-26 at 2 17 54 pm

I have not investigated any further as to what could be causing this.

@1st1
Copy link
Member

1st1 commented Jan 26, 2019

@amir20 Thank you so much.
@fantix please take a look.

@fantix
Copy link
Member

fantix commented Jan 26, 2019

Thanks for the reproducing! I'm checking into this.

@1st1
Copy link
Member

1st1 commented Jan 28, 2019

@fantix were you able to figure it out?

@fantix
Copy link
Member

fantix commented Jan 28, 2019

@1st1 I'm still trying to find a clue, but I can reproduce the issue.

@1st1
Copy link
Member

1st1 commented Jan 28, 2019

Great, thank you! I'll try to find some time to help in the next couple of days.

fantix added a commit to fantix/uvloop that referenced this issue Jan 31, 2019
fantix added a commit to fantix/uvloop that referenced this issue Jan 31, 2019
fantix added a commit to fantix/uvloop that referenced this issue Jan 31, 2019
fantix added a commit to fantix/uvloop that referenced this issue Feb 1, 2019
There were two issues: 1) at `connection_lost()` the timeout handle
was never cancelled because getting a cdef property on a cdef class
with `getattr` always return `None`, and 2) for now cancelling a
uvloop timeout handle only clears references to call arguments but
not the callback object itself, therefore the closure is now a part
of the arguments to avoid SSLProtocol object stays in memory until
the cancelled timeout handle actually get deallocated.
fantix added a commit to fantix/uvloop that referenced this issue Feb 1, 2019
There were two issues: 1) at `connection_lost()` the timeout handle
was never cancelled because getting a cdef property on a cdef class
with `getattr` always return `None`, and 2) for now cancelling a
uvloop timeout handle only clears references to call arguments but
not the callback object itself, therefore the closure is now a part
of the arguments to avoid SSLProtocol object stays in memory until
the cancelled timeout handle actually get deallocated.
fantix added a commit to fantix/uvloop that referenced this issue Feb 1, 2019
There were two issues: 1) at `connection_lost()` the timeout handle
was never cancelled because getting a cdef property on a cdef class
with `getattr` always return `None`, and 2) for now cancelling a
uvloop timeout handle only clears references to call arguments but
not the callback object itself, therefore the closure is now a part
of the arguments to avoid SSLProtocol object stays in memory until
the cancelled timeout handle actually get deallocated.
fantix added a commit to fantix/uvloop that referenced this issue Feb 2, 2019
There were two issues: 1) at `connection_lost()` the timeout handle
was never cancelled because getting a cdef property on a cdef class
with `getattr` always return `None`, and 2) for now cancelling a
uvloop timeout handle only clears references to call arguments but
not the callback object itself, therefore the closure is now a part
of the arguments to avoid SSLProtocol object stays in memory until
the cancelled timeout handle actually get deallocated.
fantix added a commit to fantix/uvloop that referenced this issue Feb 2, 2019
There were two issues: 1) at `connection_lost()` the timeout handle
was never cancelled because getting a cdef property on a cdef class
with `getattr` always return `None`, and 2) for now cancelling a
uvloop timeout handle only clears references to call arguments but
not the callback object itself, therefore the closure is now a part
of the arguments to avoid SSLProtocol object stays in memory until
the cancelled timeout handle actually get deallocated.
fantix added a commit to fantix/uvloop that referenced this issue Feb 2, 2019
There were two issues: 1) at `connection_lost()` the timeout handle
was never cancelled because getting a cdef property on a cdef class
with `getattr` always return `None`, and 2) for now cancelling a
uvloop timeout handle only clears references to call arguments but
not the callback object itself, therefore the closure is now a part
of the arguments to avoid SSLProtocol object stays in memory until
the cancelled timeout handle actually get deallocated.
fantix added a commit to fantix/uvloop that referenced this issue Feb 2, 2019
There were two issues: 1) at `connection_lost()` the timeout handle
was never cancelled because getting a cdef property on a cdef class
with `getattr` always return `None`, and 2) for now cancelling a
uvloop timeout handle only clears references to call arguments but
not the callback object itself, therefore the closure is now a part
of the arguments to avoid SSLProtocol object stays in memory until
the cancelled timeout handle actually get deallocated.
fantix added a commit to fantix/uvloop that referenced this issue Feb 11, 2019
There were two issues: 1) at `connection_lost()` the timeout handle
was never cancelled because getting a cdef property on a cdef class
with `getattr` always return `None`, and 2) for now cancelling a
uvloop timeout handle only clears references to call arguments but
not the callback object itself, therefore the closure is now a part
of the arguments to avoid SSLProtocol object stays in memory until
the cancelled timeout handle actually get deallocated.
@1st1 1st1 closed this as completed in 386fbf9 Feb 11, 2019
1st1 pushed a commit that referenced this issue Feb 11, 2019
There were two issues: 1) at `connection_lost()` the timeout handle
was never cancelled because getting a cdef property on a cdef class
with `getattr` always return `None`, and 2) for now cancelling a
uvloop timeout handle only clears references to call arguments but
not the callback object itself, therefore the closure is now a part
of the arguments to avoid SSLProtocol object stays in memory until
the cancelled timeout handle actually get deallocated.
@fantix
Copy link
Member

fantix commented Feb 11, 2019

Thanks for the merge!

@amir20 please feel free to let me know if it has further issues. Thanks!

@amir20
Copy link
Author

amir20 commented Feb 11, 2019

Nice. Would you be able to get a patch released?

@1st1
Copy link
Member

1st1 commented Feb 11, 2019

asap; I'm working on another fix right now.

@1st1
Copy link
Member

1st1 commented Feb 12, 2019

Please try the new uvloop 0.12.1; hopefully it fixes your case.

@amir20
Copy link
Author

amir20 commented Feb 13, 2019

I am testing right now. Will let it run for an hour and post an update.

@amir20
Copy link
Author

amir20 commented Feb 13, 2019

I think it works. After 30 minutes, its still at 135MB which is a good sign. Great job @1st1 :)

@1st1
Copy link
Member

1st1 commented Feb 13, 2019

I think it works. After 30 minutes, its still at 135MB which is a good sign. Great job @1st1 :)

Great news! The fix is to @fantix's credit though :)

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

No branches or pull requests

3 participants