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 #37

Closed
raulcota opened this issue Sep 7, 2018 · 4 comments
Closed

Memory leak #37

raulcota opened this issue Sep 7, 2018 · 4 comments

Comments

@raulcota
Copy link

raulcota commented Sep 7, 2018

The acquire function defines
class ReturnProxy(object):
inside its function.
This results in an obscure circular reference on the class (at least in Python 2) and memory is leaked. What leaks is the class (not the instance of the class) .

I noticed because when we run our test suite we enable gc debug . To reproduce you can do the code below. It will render the object being leaked. If you want to prove memory is indeed being leaked, just change the if 0 for if 1 and see your RAM shoot up.

For now I worked around it myself by taking the class out of the function.

from filelock import FileLock

import os
import gc

gc.enable(  )
gc.set_debug(gc.DEBUG_LEAK)

class Blah(object):
    """Reproduce leak outside of filelock"""
    def something(self):
        
        class Inner(object):
            a = range(1000000) #Make it a big class
            pass
        
        return None
    
def test():
    if 0:
        #If you want to drive your process out of memory, run 
        #this simple test (look at Task Manager in windows RAM consumption shoot up)
        for i in range(100000):
            
            #If it does not leak, then c
            #will be destroyed and cleared from memory
            c = Blah()
            c.something()
            
        
    if 1:
        lockPath = r'C:\temp\temp.txt.lock'
        lock = FileLock(lockPath)
        ret = lock.acquire()
        print 'hola'
        lock.release()
        #ret.lock = None
        #with lock:
        #    print 'hola'
    
    
    
test()

gc.collect()

print "\nGARBAGE OBJECTS:"
cnt = 0
for x in gc.garbage:
    
    cnt += 1
    if cnt > 1000:
        print "Still more garbage. Quitting ..."
        break
    try:
        s = str(x)
    except:
        s = 'ERROR TO STRING'
    print type(x),
    if len(s) > 120: s = s[:117]+'...'
    
    
    print 'o="%s"' % s
@raulcota
Copy link
Author

raulcota commented Sep 7, 2018

Not sure why code got broken down into sections but it is all one piece of code
leaktest.zip

added as zip

@AdorablePotato
Copy link
Contributor

I moved the helper class in the module namespace which seems to fix the garbage collection, but the memory usage is still quite high.

Any suggestions?

@raulcota
Copy link
Author

raulcota commented Sep 9, 2018

Thanks! I'll use your version. As a separate observation. Given that I had to review the entire code while tracking down the memory leak I noticed that you have a few calls like this,
logger().debug(...
in probably an unjustified obsession to make sure it is never a speed bottleneck, in my version of the module I have these statements like this,
if DO_LOG: logger().debug(...
and at the top of the module I have DO_LOG = True and when I do the import I flip it to False when I run it.

About "high memory" comment, not sure what you mean. If you are referring to the section,

if 0:
    #If you want to drive your process out of memory, run 
    #this simple test (look at Task Manager in windows RAM consumption shoot up)
    for i in range(100000):

then that part was just an isolated test to prove a point without using filelock.py at all.

To fix this one you'd have to do the same fix you did and move
class Inner(object):
a = range(1000000) #Make it a big class
pass
out of there.

I hope this is what you meant.

btw thanks for your effort on this module. It is extremely handy!!

@raulcota raulcota closed this as completed Sep 9, 2018
@AdorablePotato
Copy link
Contributor

I don't like the log thing either, but a user had an issue with the configuration (issue #24) and it was a small change.

I used this code to run some tests and forgot that range is not an iterator in Python 2... So nvm.

lockPath = "./my_lock"
lock = FileLock(lockPath)

lock.acquire()

for i in range(10**7):
    lock.acquire()
    lock.release()

lock.release()

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