search

celery / billiard

Multiprocessing Pool Extensions
Other
419 stars 252 forks source link

Fix rebuild_ctype losing BufferedWrapper reference on Python 3 #230

Closed Birne94 closed 7 years ago

Birne94 commented 7 years ago

Fixes #229

This is most likely a regression introduced when porting billiard to Python 3 which was for some reason uncovered.

Birne94 commented 7 years ago

Python 2.7 tests (cpython and pypy) appear to fail for some unrelated issue, values might be broken on non Python 3.

OSError: [Errno 17] File exists: '/tmp/pymp-JfdEz9/pym-2339-cp7Vsb'

listingmirror commented 7 years ago

@Birne94 Any idea how this would interact with the "CELERY_WORKER_MAX_MEMORY_PER_CHILD" property?

I just started using CELERY_WORKER_MAX_MEMORY_PER_CHILD, and I was confused what was going on. I run with 10 subprocesses. I would often hit a situation where my workers were permanently using too much memory, and would die after every task until I rebooted the container. Thinking perhaps there was something not getting GCed and it persisting across worker restart? Is that possible?

Birne94 commented 7 years ago

@listingmirror I actually do not know if CELERY_WORKER_MAX_MEMORY_PER_CHILD relies on using billiard.Value internally, but I could not find anything at first glance through the code.

While I am not too familiar with gc internals, I would guess there are some reference cycles in your code which the gc is unable to break up, resulting in the memory not being freed.

Birne94 commented 7 years ago

I think the test fails on Python 2.7 because the billiard.heap.Arena implementation appears to be buggy, so billiard.Value should be unusable from Python 2.7. This issue has been around for some time, but since billiard.Value was not tested before it never surfaced.

AppVeyor builds fail on master with the same errors, so they are unrelated.

I can have a look at it tomorrow and see if I can fix it. Not sure though if this is still in the scope of this PR.

Birne94 commented 7 years ago

I have added a fix for python2.7 and the tests for values from the cpython multiprocessing library. It appears to be working now.