Skip to content

Commit

Permalink
Fix leaked stty handle
Browse files Browse the repository at this point in the history
`os.popen` is used without closing the returned handle.
As this function is deprecated anyway replace it by `subprocess_popen_text`.
  • Loading branch information
Flamefire committed Aug 23, 2022
1 parent a8c0cad commit 4643334
Show file tree
Hide file tree
Showing 4 changed files with 10 additions and 5 deletions.
6 changes: 4 additions & 2 deletions easybuild/base/generaloption.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@

from easybuild.base.fancylogger import getLogger, setroot, setLogLevel, getDetailsLogLevels
from easybuild.base.optcomplete import autocomplete, CompleterOption
from easybuild.tools.py2vs3 import StringIO, configparser, string_type
from easybuild.tools.py2vs3 import StringIO, configparser, string_type, subprocess_popen_text
from easybuild.tools.utilities import mk_rst_table, nub, shell_quote

try:
Expand Down Expand Up @@ -80,7 +80,9 @@ def set_columns(cols=None):
stty = '/usr/bin/stty'
if os.path.exists(stty):
try:
cols = int(os.popen('%s size 2>/dev/null' % stty).read().strip().split(' ')[1])
with open(os.devnull, 'w') as devnull:
proc = subprocess_popen_text([stty, "size"], stderr=devnull)
cols = int(proc.communicate()[0].strip().split(' ')[1])
except (AttributeError, IndexError, OSError, ValueError):
# do nothing
pass
Expand Down
3 changes: 2 additions & 1 deletion easybuild/tools/py2vs3/py2.py
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,8 @@

def subprocess_popen_text(cmd, **kwargs):
"""Call subprocess.Popen with specified named arguments."""
return subprocess.Popen(cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE, **kwargs)
kwargs.setdefault('stderr', subprocess.PIPE)
return subprocess.Popen(cmd, stdout=subprocess.PIPE, **kwargs)


def raise_with_traceback(exception_class, message, traceback):
Expand Down
3 changes: 2 additions & 1 deletion easybuild/tools/py2vs3/py3.py
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,8 @@ def json_loads(body):
def subprocess_popen_text(cmd, **kwargs):
"""Call subprocess.Popen in text mode with specified named arguments."""
# open stdout/stderr in text mode in Popen when using Python 3
return subprocess.Popen(cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE, universal_newlines=True, **kwargs)
kwargs.setdefault('stderr', subprocess.PIPE)
return subprocess.Popen(cmd, stdout=subprocess.PIPE, universal_newlines=True, **kwargs)


def raise_with_traceback(exception_class, message, traceback):
Expand Down
3 changes: 2 additions & 1 deletion easybuild/tools/systemtools.py
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@
import termios
from ctypes.util import find_library
from socket import gethostname
from easybuild.tools.py2vs3 import subprocess_popen_text

# pkg_resources is provided by the setuptools Python package,
# which we really want to keep as an *optional* dependency
Expand Down Expand Up @@ -1183,7 +1184,7 @@ def det_terminal_size():
except Exception as err:
_log.warning("First attempt to determine terminal size failed: %s", err)
try:
height, width = [int(x) for x in os.popen("stty size").read().strip().split()]
height, width = [int(x) for x in subprocess_popen_text("stty size").communicate()[0].strip().split()]
except Exception as err:
_log.warning("Second attempt to determine terminal size failed, going to return defaults: %s", err)
height, width = 25, 80
Expand Down

0 comments on commit 4643334

Please sign in to comment.