- Revision
- 118979
- Author
- [email protected]
- Date
- 2012-05-30 14:58:59 -0700 (Wed, 30 May 2012)
Log Message
nrwt: seems to leak temp dirs
https://bugs.webkit.org/show_bug.cgi?id=87795
Reviewed by Ojan Vafai.
There appears to be a bug where the chromium bots are creating
temporary directories and not cleaning them up that started
after the switch to WebKitDriver. It's possible that __del__
wasn't getting called in a timely manner (or at all), and it's
generally bad style to rely on __del__ being called, so this
code changes things so that we create a temp dir in
driver.start() and remove it in driver.stop(). We could be
paranoid and leave the __del__ code in, but there doesn't seem
to be much advantage to it. If there are bugs that result in
drivers being started but not stopped, we have other problems.
* Scripts/webkitpy/common/system/filesystem_mock.py:
(MockFileSystem.__init__):
(MockFileSystem._mktemp):
(MockFileSystem.mkdtemp):
* Scripts/webkitpy/layout_tests/port/webkit.py:
(WebKitDriver.__init__):
(WebKitDriver._start):
(WebKitDriver.stop):
* Scripts/webkitpy/layout_tests/port/webkit_unittest.py:
(WebKitDriverTest.test_check_for_driver_crash):
(WebKitDriverTest):
(WebKitDriverTest.test_creating_a_port_does_not_write_to_the_filesystem):
(WebKitDriverTest.test_stop_cleans_up_properly):
Modified Paths
Diff
Modified: trunk/Tools/ChangeLog (118978 => 118979)
--- trunk/Tools/ChangeLog 2012-05-30 21:56:40 UTC (rev 118978)
+++ trunk/Tools/ChangeLog 2012-05-30 21:58:59 UTC (rev 118979)
@@ -1,3 +1,35 @@
+2012-05-30 Dirk Pranke <[email protected]>
+
+ nrwt: seems to leak temp dirs
+ https://bugs.webkit.org/show_bug.cgi?id=87795
+
+ Reviewed by Ojan Vafai.
+
+ There appears to be a bug where the chromium bots are creating
+ temporary directories and not cleaning them up that started
+ after the switch to WebKitDriver. It's possible that __del__
+ wasn't getting called in a timely manner (or at all), and it's
+ generally bad style to rely on __del__ being called, so this
+ code changes things so that we create a temp dir in
+ driver.start() and remove it in driver.stop(). We could be
+ paranoid and leave the __del__ code in, but there doesn't seem
+ to be much advantage to it. If there are bugs that result in
+ drivers being started but not stopped, we have other problems.
+
+ * Scripts/webkitpy/common/system/filesystem_mock.py:
+ (MockFileSystem.__init__):
+ (MockFileSystem._mktemp):
+ (MockFileSystem.mkdtemp):
+ * Scripts/webkitpy/layout_tests/port/webkit.py:
+ (WebKitDriver.__init__):
+ (WebKitDriver._start):
+ (WebKitDriver.stop):
+ * Scripts/webkitpy/layout_tests/port/webkit_unittest.py:
+ (WebKitDriverTest.test_check_for_driver_crash):
+ (WebKitDriverTest):
+ (WebKitDriverTest.test_creating_a_port_does_not_write_to_the_filesystem):
+ (WebKitDriverTest.test_stop_cleans_up_properly):
+
2012-05-30 Christophe Dumez <[email protected]>
[EFL] EFL's DRT should print the number of MessagePorts for new each new intent
Modified: trunk/Tools/Scripts/webkitpy/common/system/filesystem_mock.py (118978 => 118979)
--- trunk/Tools/Scripts/webkitpy/common/system/filesystem_mock.py 2012-05-30 21:56:40 UTC (rev 118978)
+++ trunk/Tools/Scripts/webkitpy/common/system/filesystem_mock.py 2012-05-30 21:58:59 UTC (rev 118979)
@@ -47,6 +47,7 @@
"""
self.files = files or {}
self.written_files = {}
+ self.last_tmpdir = None
self._sep = '/'
self.current_tmpno = 0
self.cwd = cwd
@@ -233,7 +234,8 @@
dir = self.sep + '__im_tmp'
curno = self.current_tmpno
self.current_tmpno += 1
- return self.join(dir, "%s_%u_%s" % (prefix, curno, suffix))
+ self.last_tmpdir = self.join(dir, '%s_%u_%s' % (prefix, curno, suffix))
+ return self.last_tmpdir
def mkdtemp(self, **kwargs):
class TemporaryDirectory(object):
Modified: trunk/Tools/Scripts/webkitpy/layout_tests/port/webkit.py (118978 => 118979)
--- trunk/Tools/Scripts/webkitpy/layout_tests/port/webkit.py 2012-05-30 21:56:40 UTC (rev 118978)
+++ trunk/Tools/Scripts/webkitpy/layout_tests/port/webkit.py 2012-05-30 21:58:59 UTC (rev 118979)
@@ -441,7 +441,7 @@
def __init__(self, port, worker_number, pixel_tests, no_timeout=False):
Driver.__init__(self, port, worker_number, pixel_tests, no_timeout)
- self._driver_tempdir = port._filesystem.mkdtemp(prefix='%s-' % self._port.driver_name())
+ self._driver_tempdir = None
# WebKitTestRunner can report back subprocess crashes by printing
# "#CRASHED - PROCESSNAME". Since those can happen at any time
# and ServerProcess won't be aware of them (since the actual tool
@@ -457,10 +457,9 @@
self.err_seen_eof = False
self._server_process = None
- # FIXME: This may be unsafe, as python does not guarentee any ordering of __del__ calls
- # I believe it's possible that self._port or self._port._filesystem may already be destroyed.
def __del__(self):
- self._port._filesystem.rmtree(str(self._driver_tempdir))
+ assert(self._server_process is None)
+ assert(self._driver_tempdir is None)
def cmd_line(self, pixel_tests, per_test_args):
cmd = self._command_wrapper(self._port.get_option('wrapper'))
@@ -485,6 +484,7 @@
return cmd
def _start(self, pixel_tests, per_test_args):
+ self._driver_tempdir = self._port._filesystem.mkdtemp(prefix='%s-' % self._port.driver_name())
server_name = self._port.driver_name()
environment = self._port.setup_environ_for_server(server_name)
environment['DYLD_LIBRARY_PATH'] = self._port._build_path()
@@ -668,7 +668,11 @@
self._server_process.stop()
self._server_process = None
+ if self._driver_tempdir:
+ self._port._filesystem.rmtree(str(self._driver_tempdir))
+ self._driver_tempdir = None
+
class ContentBlock(object):
def __init__(self):
self.content_type = None
Modified: trunk/Tools/Scripts/webkitpy/layout_tests/port/webkit_unittest.py (118978 => 118979)
--- trunk/Tools/Scripts/webkitpy/layout_tests/port/webkit_unittest.py 2012-05-30 21:56:40 UTC (rev 118978)
+++ trunk/Tools/Scripts/webkitpy/layout_tests/port/webkit_unittest.py 2012-05-30 21:58:59 UTC (rev 118979)
@@ -277,6 +277,7 @@
self.assertEquals(content_block.content_type, 'my_type')
self.assertEquals(content_block.encoding, 'none')
self.assertEquals(content_block.content_hash, 'foobar')
+ driver._server_process = None
def test_read_binary_block(self):
port = TestWebKitPort()
@@ -294,6 +295,7 @@
self.assertEquals(content_block.content_hash, 'actual')
self.assertEquals(content_block.content, '12345678')
self.assertEquals(content_block.decoded_content, '12345678')
+ driver._server_process = None
def test_no_timeout(self):
port = TestWebKitPort()
@@ -317,10 +319,14 @@
def has_crashed(self):
return self.crashed
+ def stop(self):
+ pass
+
def assert_crash(driver, error_line, crashed, name, pid):
self.assertEquals(driver._check_for_driver_crash(error_line), crashed)
self.assertEquals(driver._crashed_process_name, name)
self.assertEquals(driver._crashed_pid, pid)
+ driver.stop()
driver._server_process = FakeServerProcess(False)
assert_crash(driver, '', False, None, None)
@@ -344,3 +350,18 @@
driver._crashed_pid = None
driver._server_process = FakeServerProcess(True)
assert_crash(driver, '', True, 'FakeServerProcess', 1234)
+
+ def test_creating_a_port_does_not_write_to_the_filesystem(self):
+ port = TestWebKitPort()
+ driver = WebKitDriver(port, 0, pixel_tests=True)
+ self.assertEquals(port._filesystem.written_files, {})
+ self.assertEquals(port._filesystem.last_tmpdir, None)
+
+ def test_stop_cleans_up_properly(self):
+ port = TestWebKitPort()
+ driver = WebKitDriver(port, 0, pixel_tests=True)
+ driver.start(True, [])
+ last_tmpdir = port._filesystem.last_tmpdir
+ self.assertNotEquals(last_tmpdir, None)
+ driver.stop()
+ self.assertFalse(port._filesystem.isdir(last_tmpdir))
