search

ionelmc / python-redis-lock

Lock context manager implemented via redis SET NX EX and BLPOP.
https://pypi.python.org/pypi/python-redis-lock
BSD 2-Clause "Simplified" License
551 stars 77 forks source link

Make auto renewal stop after lock object destruction #33

Closed AndreiPashkin closed 8 years ago

AndreiPashkin commented 8 years ago

Subj.

ionelmc commented 8 years ago

I believe that it will never be called due to the cycle (lock holds ref to thread, thread hold ref to lock).

Maybe we should do something with weakrefs in the thread instead?

AndreiPashkin commented 8 years ago

Indeed, you right. But how to test it then?

ionelmc commented 8 years ago

Use short expiry. Then check redis directly if it's gone. Gonna be one of them slow tests ...

ionelmc commented 8 years ago

We can take a look at this after 3.0 right?

AndreiPashkin commented 8 years ago

I still have enough time, so probably yes. Don't close my pending PRs plz, I will back to them as soon as I will have time.

AndreiPashkin commented 8 years ago

I factored out InterruptableThread - because it's sole purpose was to hold reference to the semaphore, which doesn't make sense to me.

AndreiPashkin commented 8 years ago

@ionelmc, I think, the PR is ready for merging.

ionelmc commented 8 years ago

It looks like there's some failure on travis (the linux oom killer doing its thing: https://travis-ci.org/ionelmc/python-redis-lock/jobs/119551661#L2808)

AndreiPashkin commented 8 years ago

@ionelmc, wow, that's strange. Do you have any ideas?

ionelmc commented 8 years ago

Hah, I was hoping you have some. It looks like a memory leak that happens once in a while. Might be simply some unclean test code or a real bug ...

AndreiPashkin commented 8 years ago

My best guess is that "this is something to do with coverage". I also can't reproduce it (#43).