miyagawa / AnyEvent-Redis

Asynchronous Redis client
http://search.cpan.org/dist/AnyEvent-Redis
41 stars 16 forks source link

Memory Leak w/simple test case... #6

Closed jzawodn closed 14 years ago

jzawodn commented 14 years ago

I've been using the latest redis and the latest AnyEvent::Redis to try some simple examples on which I can build a larger daemon process. And I've discovered a memory leak.

I can show a trivial script that runs many GETs with no leak:

http://gist.github.com/653910

and then an almost identical script that uses BLPOP instead:

http://gist.github.com/653912

The second script will leak memory (even though it runs slower because of the timeout in BLPOP).

A co-worker who has looked at this suspects a circular reference error with $reader in anyevent_read_type in the block that starts:

} elsif($hd->{rbuf} =~ /^*/) {

That means lots of other methods are likely affected too. I'm pretty sure I hit this when messing with pubsub as well.

miyagawa commented 14 years ago

A co-worker who has looked at this suspects a circular reference error with $reader in anyevent_read_type in the block that starts:

It's not a circular reference "error" - it was intentionally made to reference itself, as a common technique used in AnyEvent. The problem here is that there's no cleanup (undef'ing it) for this $reader handler.

jzawodn commented 14 years ago

Ahh, ok. That makes sense.

Thanks for the info. I'm still working to wrap my head around AnyEvent-style programming.

miyagawa commented 14 years ago

Your co-worker is right that this circular ref is the thing that's causing the leak. We could fix it by either a) rewrite it without the circular ref (using the normal method) or b) properly cleanup the reader when 'return' from it.

jzawodn commented 14 years ago

Pull request submitted with an undef that fixes the case I tickled.

dgl commented 14 years ago

I've just pushed some changes that fix another leak and also added tests for these.

jzawodn commented 14 years ago

Excellent. Thanks!

miyagawa commented 14 years ago

0.22 shipped with the changes by dgl