fab-classic icon indicating copy to clipboard operation
fab-classic copied to clipboard

Fix a bug causing pipe FDs to leak

Open jealouscloud opened this issue 2 years ago • 3 comments

This addresses issue #51

multiprocessing.Process instances hold pipes open until their .close() method has been called, even if the child process has exited.

Manually grabbing the data from them on exit and then closing the multiprocess.Process prevents this leak.

jealouscloud avatar Apr 14 '22 19:04 jealouscloud

Tests seem to fail for python-3.6 and earlier?

   File "/__w/fab-classic/fab-classic/tests/mock_streams.py", line 74, in inner_wrapper
    func(*args, **kwargs)
  File "/__w/fab-classic/fab-classic/tests/test_network.py", line 607, in test_should_abort_when_cannot_connect
    execute(parallel_subtask, hosts=['nope.nonexistent.com'])
  File "/__w/fab-classic/fab-classic/fabric/tasks.py", line 383, in execute
    ran_jobs = jobs.run()
  File "/__w/fab-classic/fab-classic/fabric/job_queue.py", line 152, in run
    done.close()
AttributeError: 'Process' object has no attribute 'close'

ploxiln avatar Apr 15 '22 02:04 ploxiln

I tested del which seems to close the related FD as well. Added checks for multiprocessing.Process.close, which will be called on Python version that support it.

jealouscloud avatar Apr 18 '22 16:04 jealouscloud

@jealouscloud this compatibility change for Python 3.6 seems to work, great work. Although out of support it's still the default version provided by EL7 capable distros, and only Python<3.8 works with fab-classic's current parallel implementation.

Stealthii avatar Apr 26 '22 07:04 Stealthii

I am sorry I neglected this for nearly a year. Looks good - I am just going to rebase and squash before merge ...

ploxiln avatar Feb 27 '23 06:02 ploxiln