Changeset 192568 in webkit


Ignore:
Timestamp:
Nov 18, 2015 1:05:28 AM (8 years ago)
Author:
Carlos Garcia Campos
Message:

[XvfbDriver] Fail to run all layout tests when X server started with -displayfd option
https://bugs.webkit.org/show_bug.cgi?id=151135

Reviewed by Darin Adler.

The XvfbDriver uses the x server command line to check the
displays that are currently in use. This doesn't work when X
server was started with -displayfd option. This option is used to let
the server find the display id available that is written to the
given file descriptor. With this option xorg doesn't need to
create the lock files in tmp either. The -displayfd option is also
available in Xvfb, so we could use it too. That would simplify the
code, fixing also race conditions between the check for available
displays and Xvfb opening the connection, we wouldn't need to wait
for 4 seconds after launching Xvfb, and all lock files we are
using won't be needed either.

  • Scripts/webkitpy/port/xvfbdriver.py:

(XvfbDriver._xvfb_pipe): Helper function to create the pipe, only
needed to be overriden by unit tests.
(XvfbDriver._xvfb_read_display_id): Helper function to read from
the pipe, only needed to be overriden by unit tests.
(XvfbDriver._xvfb_run): Run Xvfb with -displayfd option, using a
pipe to read the display id.
(XvfbDriver._start): Call _xvfb_run() and remove the code to run
Xvfb for a given display.
(XvfbDriver.stop): Remove code to release and delete file locks.

  • Scripts/webkitpy/port/xvfbdriver_unittest.py:

(XvfbDriverTest.make_driver):
(XvfbDriverTest.test_start):
(XvfbDriverTest.test_start_arbitrary_worker_number):
(XvfbDriverTest.test_stop):
(XvfbDriverTest.assertDriverStartSuccessful): Deleted.
(XvfbDriverTest): Deleted.
(XvfbDriverTest.test_stop.FakeXvfbProcess): Deleted.

Location:
trunk/Tools
Files:
3 edited

Legend:

Unmodified
Added
Removed
  • trunk/Tools/ChangeLog

    r192565 r192568  
     12015-11-18  Carlos Garcia Campos  <cgarcia@igalia.com>
     2
     3        [XvfbDriver] Fail to run all layout tests when X server started with -displayfd option
     4        https://bugs.webkit.org/show_bug.cgi?id=151135
     5
     6        Reviewed by Darin Adler.
     7
     8        The XvfbDriver uses the x server command line to check the
     9        displays that are currently in use. This doesn't work when X
     10        server was started with -displayfd option. This option is used to let
     11        the server find the display id available that is written to the
     12        given file descriptor. With this option xorg doesn't need to
     13        create the lock files in tmp either. The -displayfd option is also
     14        available in Xvfb, so we could use it too. That would simplify the
     15        code, fixing also race conditions between the check for available
     16        displays and Xvfb opening the connection, we wouldn't need to wait
     17        for 4 seconds after launching Xvfb, and all lock files we are
     18        using won't be needed either.
     19
     20        * Scripts/webkitpy/port/xvfbdriver.py:
     21        (XvfbDriver._xvfb_pipe): Helper function to create the pipe, only
     22        needed to be overriden by unit tests.
     23        (XvfbDriver._xvfb_read_display_id): Helper function to read from
     24        the pipe, only needed to be overriden by unit tests.
     25        (XvfbDriver._xvfb_run): Run Xvfb with -displayfd option, using a
     26        pipe to read the display id.
     27        (XvfbDriver._start): Call _xvfb_run() and remove the code to run
     28        Xvfb for a given display.
     29        (XvfbDriver.stop): Remove code to release and delete file locks.
     30        * Scripts/webkitpy/port/xvfbdriver_unittest.py:
     31        (XvfbDriverTest.make_driver):
     32        (XvfbDriverTest.test_start):
     33        (XvfbDriverTest.test_start_arbitrary_worker_number):
     34        (XvfbDriverTest.test_stop):
     35        (XvfbDriverTest.assertDriverStartSuccessful): Deleted.
     36        (XvfbDriverTest): Deleted.
     37        (XvfbDriverTest.test_stop.FakeXvfbProcess): Deleted.
     38
    1392015-11-17  Alexey Proskuryakov  <ap@apple.com>
    240
  • trunk/Tools/Scripts/webkitpy/port/xvfbdriver.py

    r182566 r192568  
    3535from webkitpy.port.server_process import ServerProcess
    3636from webkitpy.port.driver import Driver
    37 from webkitpy.common.system.file_lock import FileLock
    3837
    3938_log = logging.getLogger(__name__)
     
    5150        return xvfb_found
    5251
    53     def __init__(self, *args, **kwargs):
    54         Driver.__init__(self, *args, **kwargs)
    55         self._guard_lock = None
    56         self._startup_delay_secs = 1.0
     52    def _xvfb_pipe(self):
     53        return os.pipe()
    5754
    58     def _next_free_display(self):
    59         running_pids = self._port.host.executive.run_command(['ps', '-eo', 'comm,command'])
    60         reserved_screens = set()
    61         for pid in running_pids.split('\n'):
    62             match = re.match('(X|Xvfb|Xorg|Xorg\.bin)\s+.*\s:(?P<screen_number>\d+)', pid)
    63             if match:
    64                 reserved_screens.add(int(match.group('screen_number')))
    65         for i in range(1, 99):
    66             if i not in reserved_screens:
    67                 _guard_lock_file = self._port.host.filesystem.join('/tmp', 'WebKitXvfb.lock.%i' % i)
    68                 self._guard_lock = self._port.host.make_file_lock(_guard_lock_file)
    69                 if self._guard_lock.acquire_lock():
    70                     return i
     55    def _xvfb_read_display_id(self, read_fd):
     56        import errno
     57        import select
     58
     59        fd_set = [read_fd]
     60        while fd_set:
     61            try:
     62                fd_list = select.select(fd_set, [], [])[0]
     63            except select.error, e:
     64                if e.args[0] == errno.EINTR:
     65                    continue
     66                raise
     67
     68            if read_fd in fd_list:
     69                # We only expect a number, so first read should be enough.
     70                display_id = os.read(read_fd, 256).strip('\n')
     71                fd_set = []
     72
     73        return int(display_id)
     74
     75    def _xvfb_run(self, environment):
     76        read_fd, write_fd = self._xvfb_pipe()
     77        run_xvfb = ["Xvfb", "-displayfd", str(write_fd), "-screen",  "0", "1024x768x%s" % self._xvfb_screen_depth(), "-nolisten", "tcp"]
     78        if self._port._should_use_jhbuild():
     79            run_xvfb = self._port._jhbuild_wrapper + run_xvfb
     80        with open(os.devnull, 'w') as devnull:
     81            self._xvfb_process = self._port.host.executive.popen(run_xvfb, stderr=devnull, env=environment)
     82            display_id = self._xvfb_read_display_id(read_fd)
     83
     84        try:
     85            os.close(read_fd)
     86            os.close(write_fd)
     87        except OSError:
     88            pass
     89
     90        return display_id
    7191
    7292    def _xvfb_screen_depth(self):
     
    7696        self.stop()
    7797
    78         # Use even displays for pixel tests and odd ones otherwise. When pixel tests are disabled,
    79         # DriverProxy creates two drivers, one for normal and the other for ref tests. Both have
    80         # the same worker number, so this prevents them from using the same Xvfb instance.
    81         display_id = self._next_free_display()
    82         self._lock_file = "/tmp/.X%d-lock" % display_id
    83 
    8498        server_name = self._port.driver_name()
    8599        environment = self._port.setup_environ_for_server(server_name)
    86 
    87         run_xvfb = ["Xvfb", ":%d" % display_id, "-screen",  "0", "1024x768x%s" % self._xvfb_screen_depth(), "-nolisten", "tcp"]
    88 
    89         if self._port._should_use_jhbuild():
    90                 run_xvfb = self._port._jhbuild_wrapper + run_xvfb
    91 
    92         with open(os.devnull, 'w') as devnull:
    93             self._xvfb_process = self._port.host.executive.popen(run_xvfb, stderr=devnull, env=environment)
    94 
    95         # Crashes intend to occur occasionally in the first few tests that are run through each
    96         # worker because the Xvfb display isn't ready yet. Halting execution a bit should avoid that.
    97         time.sleep(self._startup_delay_secs)
     100        display_id = self._xvfb_run(environment)
    98101
    99102        # We must do this here because the DISPLAY number depends on _worker_number
     
    116119    def stop(self):
    117120        super(XvfbDriver, self).stop()
    118         if self._guard_lock:
    119             self._guard_lock.release_lock()
    120             self._guard_lock = None
    121121        if getattr(self, '_xvfb_process', None):
    122122            self._port.host.executive.kill_process(self._xvfb_process.pid)
    123123            self._xvfb_process = None
    124             if self._port.host.filesystem.exists(self._lock_file):
    125                 self._port.host.filesystem.remove(self._lock_file)
  • trunk/Tools/Scripts/webkitpy/port/xvfbdriver_unittest.py

    r182566 r192568  
    5353        driver._startup_delay_secs = 0
    5454        driver._xvfb_screen_depth = lambda: '24'
     55        driver._xvfb_pipe = lambda: (3, 4)
     56        driver._xvfb_read_display_id = lambda(x): 1
    5557        driver._environment = port.setup_environ_for_server(port.driver_name())
    5658        return driver
     
    6769        self.assertEqual(driver._server_process.env["DISPLAY"], expected_display)
    6870
    69     def test_start_no_pixel_tests(self):
     71    def test_start(self):
    7072        driver = self.make_driver()
    71         expected_logs = ("MOCK run_command: ['ps', '-eo', 'comm,command'], cwd=None\nMOCK popen: ['Xvfb', ':1', '-screen', '0', '1024x768x24', '-nolisten', 'tcp'], env=%s\n" % driver._environment)
     73        expected_logs = ("MOCK popen: ['Xvfb', '-displayfd', '4', '-screen', '0', '1024x768x24', '-nolisten', 'tcp'], env=%s\n" % driver._environment)
    7274        self.assertDriverStartSuccessful(driver, expected_logs=expected_logs, expected_display=":1")
    73         self.cleanup_driver(driver)
    74 
    75     def test_start_pixel_tests(self):
    76         driver = self.make_driver()
    77         expected_logs = ("MOCK run_command: ['ps', '-eo', 'comm,command'], cwd=None\nMOCK popen: ['Xvfb', ':1', '-screen', '0', '1024x768x24', '-nolisten', 'tcp'], env=%s\n" % driver._environment)
    78         self.assertDriverStartSuccessful(driver, expected_logs=expected_logs, expected_display=":1", pixel_tests=True)
    7975        self.cleanup_driver(driver)
    8076
    8177    def test_start_arbitrary_worker_number(self):
    8278        driver = self.make_driver(worker_number=17)
    83         expected_logs = ("MOCK run_command: ['ps', '-eo', 'comm,command'], cwd=None\nMOCK popen: ['Xvfb', ':1', '-screen', '0', '1024x768x24', '-nolisten', 'tcp'], env=%s\n" % driver._environment)
     79        expected_logs = ("MOCK popen: ['Xvfb', '-displayfd', '4', '-screen', '0', '1024x768x24', '-nolisten', 'tcp'], env=%s\n" % driver._environment)
    8480        self.assertDriverStartSuccessful(driver, expected_logs=expected_logs, expected_display=":1", pixel_tests=True)
    8581        self.cleanup_driver(driver)
    8682
    87     def test_next_free_display(self):
    88         output = "Xorg            /usr/bin/X :1 -auth /var/run/lightdm/root/:1 -nolisten tcp vt7 -novtswitch -background none\nXvfb            Xvfb :2 -screen 0 800x600x24 -nolisten tcp"
    89         executive = MockExecutive2(output)
    90         driver = self.make_driver(executive=executive)
    91         self.assertEqual(driver._next_free_display(), 3)
    92         self.cleanup_driver(driver)
    93         output = "X               /usr/bin/X :1 vt7 -nolisten tcp -auth /var/run/xauth/A:0-8p7Ybb"
    94         executive = MockExecutive2(output)
    95         driver = self.make_driver(executive=executive)
    96         self.assertEqual(driver._next_free_display(), 2)
    97         self.cleanup_driver(driver)
    98         output = "Xvfb            Xvfb :1 -screen 0 800x600x24 -nolisten tcp"
    99         executive = MockExecutive2(output)
    100         driver = self.make_driver(executive=executive)
    101         self.assertEqual(driver._next_free_display(), 2)
    102         self.cleanup_driver(driver)
    103         output = "Xvfb            Xvfb :2 -screen 0 800x600x24 -nolisten tcp\nXvfb            Xvfb :1 -screen 0 800x600x24 -nolisten tcp\nXvfb            Xvfb :3 -screen 0 800x600x24 -nolisten tcp"
    104         executive = MockExecutive2(output)
    105         driver = self.make_driver(executive=executive)
    106         self.assertEqual(driver._next_free_display(), 4)
    107         self.cleanup_driver(driver)
    108         output = "Xorg.bin        /usr/libexec/Xorg.bin :1 -background none -noreset -verbose 3 -logfile /dev/null -auth /run/gdm/auth-for-gdm-Zt8Eq2/database -seat seat0 -nolisten tcp vt1"
    109         executive = MockExecutive2(output)
    110         driver = self.make_driver(executive=executive)
    111         self.assertEqual(driver._next_free_display(), 2)
    112         self.cleanup_driver(driver)
    113         output = "/usr/libexec/Xorg            vt2 -displayfd 3 -auth /run/user/1000/gdm/Xauthority -nolisten tcp -background none -noreset -keeptty -verbose 3"
    114         executive = MockExecutive2(output)
    115         driver = self.make_driver(executive=executive)
    116         self.assertEqual(driver._next_free_display(), 1)
    117         self.cleanup_driver(driver)
    118 
    119     def test_start_next_worker(self):
    120         driver = self.make_driver()
    121         driver._next_free_display = lambda: 1
    122         expected_logs = ("MOCK popen: ['Xvfb', ':1', '-screen', '0', '1024x768x24', '-nolisten', 'tcp'], env=%s\n" % driver._environment)
    123         self.assertDriverStartSuccessful(driver, expected_logs=expected_logs, expected_display=":1", pixel_tests=True)
    124         self.cleanup_driver(driver)
    125         driver = self.make_driver()
    126         driver._next_free_display = lambda: 3
    127         expected_logs = ("MOCK popen: ['Xvfb', ':3', '-screen', '0', '1024x768x24', '-nolisten', 'tcp'], env=%s\n" % driver._environment)
    128         self.assertDriverStartSuccessful(driver, expected_logs=expected_logs, expected_display=":3", pixel_tests=True)
    129         self.cleanup_driver(driver)
    130 
    13183    def test_stop(self):
    132         filesystem = MockFileSystem(files={'/tmp/.X42-lock': '1234\n'})
    133         port = Port(MockSystemHost(log_executive=True, filesystem=filesystem), 'xvfbdrivertestport', options=MockOptions(configuration='Release'))
     84        port = Port(MockSystemHost(log_executive=True), 'xvfbdrivertestport', options=MockOptions(configuration='Release'))
    13485        port._executive.kill_process = lambda x: _log.info("MOCK kill_process pid: " + str(x))
    13586        driver = XvfbDriver(port, worker_number=0, pixel_tests=True)
     
    13990
    14091        driver._xvfb_process = FakeXvfbProcess()
    141         driver._lock_file = '/tmp/.X42-lock'
    14292
    14393        expected_logs = "MOCK kill_process pid: 1234\n"
     
    14595
    14696        self.assertIsNone(driver._xvfb_process)
    147         self.assertFalse(port._filesystem.exists(driver._lock_file))
Note: See TracChangeset for help on using the changeset viewer.