search

cgoldberg / xvfbwrapper

Manage headless displays with Xvfb (X virtual framebuffer)
Other
295 stars 52 forks source link

Do not pass open file descriptors to Xvfb #10

Closed koterpillar closed 8 years ago

koterpillar commented 8 years ago

Explicitly pass close_fds=True so that Xvfb does not inherit the open file descriptors of the calling process.

cgoldberg commented 8 years ago

@koterpillar

Thanks for the patch!

I noticed that file descriptor behavior for subprocess.Popen was changed in Python 3.3, and close_fds=True is the new default. So your change would make Python2 and Python3 function the same.

However, I also read there is a performance penalty for using close_fds=True. see: https://www.python.org/dev/peps/pep-0446/

Before I merge this patch, could you describe the problem that this solves for Python 2.7? Do the file descriptors leak and continue to grow? (PEP 0446 is pretty confusing to me). I'd like to understand the current issue better before merging.

Thanks!

koterpillar commented 8 years ago

The issue I'm having is, the Python side is opening a TCP port before starting Xvfb (Django test server), then closes and reopens it for the next test. However, because Xvfb has inherited the port, it can't be closed so the next test fails with "Address already in use".

This is actually the second issue in https://www.python.org/dev/peps/pep-0446/#id9.

cgoldberg commented 8 years ago

lgtm. thanks much! sorry for the merge delay... holidays and all :)

cgoldberg commented 8 years ago

I just released the latest to PyPI including your change: https://pypi.python.org/pypi/xvfbwrapper/0.2.7

koterpillar commented 8 years ago

Thanks!