From 5c92b6e56d52be6a50c2c6942ab14287ceb14737 Mon Sep 17 00:00:00 2001 From: Elias Ohm Date: Sat, 20 Oct 2018 12:22:55 +0200 Subject: [PATCH 1/3] add argument for reliably checking the started X server is our own - needed to avoid race conditions when multiple scripts start a X server simultaniously - currently the python 3 implementation is possibly incomplete as the EasyProcess module currently does not provide a way to inherit the needed fd on pathon >= 3.2 --- pyvirtualdisplay/abstractdisplay.py | 33 ++++++++++++++++++++++++++--- pyvirtualdisplay/display.py | 6 ++++-- pyvirtualdisplay/xephyr.py | 2 ++ pyvirtualdisplay/xvfb.py | 2 ++ pyvirtualdisplay/xvnc.py | 2 ++ 5 files changed, 40 insertions(+), 5 deletions(-) diff --git a/pyvirtualdisplay/abstractdisplay.py b/pyvirtualdisplay/abstractdisplay.py index 1796ca5..3a53c54 100644 --- a/pyvirtualdisplay/abstractdisplay.py +++ b/pyvirtualdisplay/abstractdisplay.py @@ -5,6 +5,8 @@ import time import tempfile from threading import Lock +import select +import fcntl from pyvirtualdisplay import xauth @@ -22,7 +24,7 @@ MIN_DISPLAY_NR = 1000 USED_DISPLAY_NR_LIST=[] -X_START_TIMEOUT = 1 +X_START_TIMEOUT = 10 X_START_TIME_STEP = 0.1 X_START_WAIT = 0.1 @@ -34,7 +36,7 @@ class AbstractDisplay(EasyProcess): Common parent for Xvfb and Xephyr ''' - def __init__(self, use_xauth=False): + def __init__(self, use_xauth=False, check_startup=False): mutex.acquire() try: self.display = self.search_for_display() @@ -48,6 +50,15 @@ def __init__(self, use_xauth=False): self.use_xauth = use_xauth self._old_xauth = None self._xauth_filename = None + self.check_startup = check_startup + if self.check_startup: + rp, wp = os.pipe() + fcntl.fcntl(rp, fcntl.F_SETFD, fcntl.FD_CLOEXEC) + #to properly allow to inherit fds to subprocess on + #python 3.2+ the easyprocess needs small fix.. + fcntl.fcntl(wp, fcntl.F_SETFD, 0) + self.check_startup_fd = wp + self._check_startup_fd = rp EasyProcess.__init__(self, self._cmd) @property @@ -115,8 +126,24 @@ def start(self): # wait until X server is active start_time = time.time() - ok = False + if self.check_startup: + rp = self._check_startup_fd + display_check = None + rlist, wlist, xlist = select.select((rp,), (), (), X_START_TIMEOUT) + if rlist: + display_check = os.read(rp, 10).rstrip() + else: + msg = 'No display number returned by X server' + raise XStartTimeoutError(msg) + dnbs = str(self.display) + if bytes != str: + dnbs = bytes(dnbs, 'ascii') + if display_check != dnbs: + msg = 'Display number "%s" not returned by X server' + str(display_check) + raise XStartTimeoutError(msg % self.display) + d = self.new_display_var + ok = False while time.time() - start_time < X_START_TIMEOUT: try: exit_code = EasyProcess('xdpyinfo').call().return_code diff --git a/pyvirtualdisplay/display.py b/pyvirtualdisplay/display.py index a799058..539714d 100644 --- a/pyvirtualdisplay/display.py +++ b/pyvirtualdisplay/display.py @@ -15,7 +15,7 @@ class Display(AbstractDisplay): :param backend: 'xvfb', 'xvnc' or 'xephyr', ignores ``visible`` :param xauth: If a Xauthority file should be created. ''' - def __init__(self, backend=None, visible=False, size=(1024, 768), color_depth=24, bgcolor='black', use_xauth=False, **kwargs): + def __init__(self, backend=None, visible=False, size=(1024, 768), color_depth=24, bgcolor='black', use_xauth=False, check_startup=False, **kwargs): self.color_depth = color_depth self.size = size self.bgcolor = bgcolor @@ -36,7 +36,7 @@ def __init__(self, backend=None, visible=False, size=(1024, 768), color_depth=24 color_depth=color_depth, bgcolor=bgcolor, **kwargs) - AbstractDisplay.__init__(self, use_xauth=use_xauth) + AbstractDisplay.__init__(self, use_xauth=use_xauth, check_startup=check_startup) @property def display_class(self): @@ -56,4 +56,6 @@ def display_class(self): @property def _cmd(self): self._obj.display = self.display + self._obj.check_startup = self.check_startup + self._obj.check_startup_fd = self.check_startup_fd return self._obj._cmd diff --git a/pyvirtualdisplay/xephyr.py b/pyvirtualdisplay/xephyr.py index ef6a919..60fcd83 100644 --- a/pyvirtualdisplay/xephyr.py +++ b/pyvirtualdisplay/xephyr.py @@ -38,4 +38,6 @@ def _cmd(self): '-resizeable', self.new_display_var, ] + if self.check_startup: + cmd += ['-displayfd', str(self.check_startup_fd)] return cmd diff --git a/pyvirtualdisplay/xvfb.py b/pyvirtualdisplay/xvfb.py index fa17eab..0a31b81 100644 --- a/pyvirtualdisplay/xvfb.py +++ b/pyvirtualdisplay/xvfb.py @@ -50,4 +50,6 @@ def _cmd(self): ] if self.fbdir: cmd += ['-fbdir', self.fbdir] + if self.check_startup: + cmd += ['-displayfd', str(self.check_startup_fd)] return [PROGRAM] + cmd diff --git a/pyvirtualdisplay/xvnc.py b/pyvirtualdisplay/xvnc.py index b96e21f..e43cea2 100644 --- a/pyvirtualdisplay/xvnc.py +++ b/pyvirtualdisplay/xvnc.py @@ -40,4 +40,6 @@ def _cmd(self): '-rfbport', str(self.rfbport), self.new_display_var, ] + if self.check_startup: + cmd += ['-displayfd', str(self.check_startup_fd)] return cmd From 9164e94f0e96029d837fcb13ca6c9be42475e68f Mon Sep 17 00:00:00 2001 From: Elias Ohm Date: Sat, 20 Oct 2018 12:34:04 +0200 Subject: [PATCH 2/3] fix for untested case check_startup=False... --- pyvirtualdisplay/display.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/pyvirtualdisplay/display.py b/pyvirtualdisplay/display.py index 539714d..7052388 100644 --- a/pyvirtualdisplay/display.py +++ b/pyvirtualdisplay/display.py @@ -57,5 +57,6 @@ def display_class(self): def _cmd(self): self._obj.display = self.display self._obj.check_startup = self.check_startup - self._obj.check_startup_fd = self.check_startup_fd + if self.check_startup: + self._obj.check_startup_fd = self.check_startup_fd return self._obj._cmd From 94d96ee45a210dde7bc55494cb68134f8fad92ea Mon Sep 17 00:00:00 2001 From: Elias Ohm Date: Sat, 20 Oct 2018 13:19:03 +0200 Subject: [PATCH 3/3] account for possible race on slow X startup (or unlikely slow pipe reading..) with check_startup=True - if the select on displayfd has to wait to nearly as long as startup-timeout, the following xdpyinfo check block could be skipped causing the startup to fail without real reason because success not set by that block --- pyvirtualdisplay/abstractdisplay.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/pyvirtualdisplay/abstractdisplay.py b/pyvirtualdisplay/abstractdisplay.py index 3a53c54..a65630d 100644 --- a/pyvirtualdisplay/abstractdisplay.py +++ b/pyvirtualdisplay/abstractdisplay.py @@ -144,7 +144,7 @@ def start(self): d = self.new_display_var ok = False - while time.time() - start_time < X_START_TIMEOUT: + while True: try: exit_code = EasyProcess('xdpyinfo').call().return_code except EasyProcessError: @@ -160,6 +160,8 @@ def start(self): ok = True break + if time.time() - start_time >= X_START_TIMEOUT: + break time.sleep(X_START_TIME_STEP) if not ok: msg = 'Failed to start X on display "%s" (xdpyinfo check failed).'