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

Fixed several memory leaks #62

Merged
merged 2 commits into from Oct 1, 2012

Conversation

3 participants
@mlindgren
Contributor

mlindgren commented Sep 30, 2012

I found and fixed more than 10 memory leaks in Wand in this patch. Specifically:

  • There was a circular reference between the Image and Metadata classes. Python cannot garbage collect circular references on classes which have explicit destructors, so this meant that Images were never freed. I fixed this by changing the reference from Metadata to its parent Image to a weakref.

    Without patch:

    >>> from wand.image import Image                                                    
    >>> import gc                                                                       
    >>> def f():                                                                        
    ...     img = Image(filename='/Users/mitch/Pictures/mitch_kh.jpg')                  
    ...                                                                                 
    >>> for i in range(10): f()                                                         
    ...                                                                                 
    >>> gc.collect()                                                                    
    40                                                                                  
    >>> gc.garbage                                                                      
    [<wand.image.Image object at 0x103197d40>, <wand.image.Image object at              
    0x103197e60>, <wand.image.Image object at 0x103197ea8>, <wand.image.Image object 
    at 0x103197ef0>, <wand.image.Image object at 0x103197f38>, <wand.image.Image        
    object at 0x103197f80>, <wand.image.Image object at 0x103197fc8>,                   
    <wand.image.Image object at 0x10319e050>, <wand.image.Image object at               
    0x10319e098>, <wand.image.Image object at 0x10319e0e0>]                             

    With patch:

    >>> from wand.image import Image                                                    
    >>> import gc                                                                       
    >>> def f():                                                                        
    ...     img = Image(filename='/Users/mitch/Pictures/mitch_kh.jpg')                  
    ...                                                                                 
    >>> for i in range(10): f()                                                         
    ...                                                                                 
    >>> gc.collect()                                                                    
    0                                                                                   
    >>> gc.garbage                                                                      
    []                                                                     
  • The setter for Resource did not destroy the previous c_resource, if any existed, which caused resources to be leaked when an Image's wand property was changed after creation.

  • Almost all of the ctypes functions in api.py returning c_char_ps leaked, because c_char_ps are automatically converted to Python strings and the original pointer is discarded, making it impossible to free the C string with MagickRelinquishMemory. I fixed this by subclassing c_char_p to avoid the automatic conversion and accesing .value in places where the string values were previously used, and calling MagickRelinquishMemory in c_magick_char_p's destructor.

    GetMagickVersion and GetMagickReleaseDate didn't need to be changed because they return const strings.

I ran the tests with these changes on Windows 7 x64, CentOS 6.3 x64 and OS X Mountain Lion x64. I also did some impromptu memory testing by creating images and performing operations on them in a loop which ran hundreds of times. I think I've gotten rid of all of the leaks but it's hard to be sure because I don't know of a good tool to find ctypes leaks.

I did not change the version number—I'll leave that up to you, but I would strongly recommend releasing this patch before version 3.0 as some of the leaks I fixed can very quickly lead to Python consuming all of a machine's RAM. I noticed the problem at first when the web app I'm developing was using 2GB of RAM after I uploaded some photos. :)

mlindgren added some commits Sep 30, 2012

Removed unnecessary __enter__ and __exit__ functions from c_magick_ch…
…ar_p (the latter was incorrect anyway; added extra information to docstrings.
@@ -1,3 +1,4 @@
# -*- coding: utf-8 -*-

This comment has been minimized.

@mlindgren

mlindgren Sep 30, 2012

Contributor

I added this to this file because in my last patch I put some UTF-8 characters in the transform test, and some versions of Python will complain if there are Unicode characters in a file but no encoding declaration. My bad.

@dahlia

This comment has been minimized.

Collaborator

dahlia commented Oct 1, 2012

Really thanks for your effort! Although we’ve been aware of some of memory leaks, we hadn’t done nothing. You did very important work for us! :-) I merge this right now.

dahlia added a commit that referenced this pull request Oct 1, 2012

Merge pull request #62 from mlindgren/master
Fixed several memory leaks

@dahlia dahlia merged commit f449505 into emcconville:master Oct 1, 2012

1 check passed

default The Travis build passed
Details

dahlia added a commit that referenced this pull request Oct 1, 2012

dahlia added a commit that referenced this pull request Apr 17, 2013

@cgfactory

This comment has been minimized.

cgfactory commented Jul 7, 2014

This doesn't fix memory leaks, when using transform method.
wand version 0.3.7
ImageMagick version 6.6.04

Executing code from this issue #104, prints constantly increasing memory usage!

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