When the driver is stopped (when it finishes running all tests or some test timeout/crash), following exception raises and layout test script quits: ValueError raised: I/O operation on closed file File "/usr/local/google/home/wangxianzhu/webkit/Tools/Scripts/webkitpy/layout_tests/run_webkit_tests.py", line 110, in run unexpected_result_count = manager.run(args) File "/usr/local/google/home/wangxianzhu/webkit/Tools/Scripts/webkitpy/layout_tests/controllers/manager.py", line 411, in run self._run_tests(self._test_names, result_summary, int(self._options.child_processes)) File "/usr/local/google/home/wangxianzhu/webkit/Tools/Scripts/webkitpy/layout_tests/controllers/manager.py", line 464, in _run_tests return self._runner.run_tests(test_inputs, self._expectations, result_summary, num_workers, needs_http, needs_websockets, self._retrying) File "/usr/local/google/home/wangxianzhu/webkit/Tools/Scripts/webkitpy/layout_tests/controllers/layout_test_runner.py", line 142, in run_tests pool.run(('test_list', shard.name, shard.test_inputs) for shard in all_shards) File "/usr/local/google/home/wangxianzhu/webkit/Tools/Scripts/webkitpy/common/message_pool.py", line 97, in run self.wait() File "/usr/local/google/home/wangxianzhu/webkit/Tools/Scripts/webkitpy/common/message_pool.py", line 127, in wait self._workers[0].run() File "/usr/local/google/home/wangxianzhu/webkit/Tools/Scripts/webkitpy/common/message_pool.py", line 267, in run self._raise(sys.exc_info()) File "/usr/local/google/home/wangxianzhu/webkit/Tools/Scripts/webkitpy/common/message_pool.py", line 255, in run worker.handle(message.name, message.src, *message.args) File "/usr/local/google/home/wangxianzhu/webkit/Tools/Scripts/webkitpy/layout_tests/controllers/layout_test_runner.py", line 304, in handle self._run_test(test_input) File "/usr/local/google/home/wangxianzhu/webkit/Tools/Scripts/webkitpy/layout_tests/controllers/layout_test_runner.py", line 335, in _run_test self._clean_up_after_test(test_input, result) File "/usr/local/google/home/wangxianzhu/webkit/Tools/Scripts/webkitpy/layout_tests/controllers/layout_test_runner.py", line 381, in _clean_up_after_test self._kill_driver() File "/usr/local/google/home/wangxianzhu/webkit/Tools/Scripts/webkitpy/layout_tests/controllers/layout_test_runner.py", line 367, in _kill_driver driver.stop() File "/usr/local/google/home/wangxianzhu/webkit/Tools/Scripts/webkitpy/layout_tests/port/chromium_android.py", line 645, in stop self._read_stdout_process.kill() File "/usr/local/google/home/wangxianzhu/webkit/Tools/Scripts/webkitpy/layout_tests/port/server_process.py", line 362, in kill self.stop(0.0) File "/usr/local/google/home/wangxianzhu/webkit/Tools/Scripts/webkitpy/layout_tests/port/server_process.py", line 352, in stop self._wait_for_data_and_update_buffers_using_select(now, stopping=True) File "/usr/local/google/home/wangxianzhu/webkit/Tools/Scripts/webkitpy/layout_tests/port/server_process.py", line 229, in _wait_for_data_and_update_buffers_using_select out_fd = self._proc.stdout.fileno() Exception ValueError: ValueError('I/O operation on closed file',) in <bound method ChromiumAndroidDriver.__del__ of <webkitpy.layout_tests.port.chromium_android.ChromiumAndroidDriver object at 0x37e8f10>> ignored
Created attachment 168254 [details] Patch
Comment on attachment 168254 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=168254&action=review This seems reasonable. One > Tools/Scripts/webkitpy/layout_tests/port/server_process.py:334 > self._proc.stdin.close() Can you add a "self._proc.stdin = None" here as well? I think w/o this we can end up trying to close the handle twice in self._reset() on line 357 (this isn't your fault, but I noticed this while reviewing the patch).
(In reply to comment #2) > (From update of attachment 168254 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=168254&action=review > > This seems reasonable. One > > > Tools/Scripts/webkitpy/layout_tests/port/server_process.py:334 > > self._proc.stdin.close() > > Can you add a "self._proc.stdin = None" here as well? I think w/o this we can end up trying to close the handle twice in self._reset() on line 357 (this isn't your fault, but I noticed this while reviewing the patch). The problem seems more complex than I thought after I noticed this line. For all ports in the later _wait_for_data_and_update_buffers_using_select() stdin.fileno() will be called while stdin has been closed. On chromium-android, this causes the exception (though the situation is a bit different). I wonder what the intent of the line is. Can we just remove it?
(In reply to comment #3) > (In reply to comment #2) > > (From update of attachment 168254 [details] [details]) > > View in context: https://bugs.webkit.org/attachment.cgi?id=168254&action=review > > > > This seems reasonable. One > > > > > Tools/Scripts/webkitpy/layout_tests/port/server_process.py:334 > > > self._proc.stdin.close() > > > > Can you add a "self._proc.stdin = None" here as well? I think w/o this we can end up trying to close the handle twice in self._reset() on line 357 (this isn't your fault, but I noticed this while reviewing the patch). > > The problem seems more complex than I thought after I noticed this line. For all ports in the later _wait_for_data_and_update_buffers_using_select() stdin.fileno() will be called while stdin has been closed. On chromium-android, this causes the exception (though the situation is a bit different). > > I wonder what the intent of the line is. Can we just remove it? closing stdin is the way DRT is supposed to be notified to shut down cleanly, so, no. But, _wait_for_data doesn't access stdin? Did you misread something?
(In reply to comment #4) > (In reply to comment #3) > > (In reply to comment #2) > > > (From update of attachment 168254 [details] [details] [details]) > > > View in context: https://bugs.webkit.org/attachment.cgi?id=168254&action=review > > > > > > This seems reasonable. One > > > > > > > Tools/Scripts/webkitpy/layout_tests/port/server_process.py:334 > > > > self._proc.stdin.close() > > > > > > Can you add a "self._proc.stdin = None" here as well? I think w/o this we can end up trying to close the handle twice in self._reset() on line 357 (this isn't your fault, but I noticed this while reviewing the patch). > > > > The problem seems more complex than I thought after I noticed this line. For all ports in the later _wait_for_data_and_update_buffers_using_select() stdin.fileno() will be called while stdin has been closed. On chromium-android, this causes the exception (though the situation is a bit different). > > > > I wonder what the intent of the line is. Can we just remove it? > > closing stdin is the way DRT is supposed to be notified to shut down cleanly, so, no. > > But, _wait_for_data doesn't access stdin? Did you misread something? Yes, I mixed up stdin and stdout :)
Created attachment 168270 [details] Patch
Comment on attachment 168270 [details] Patch Clearing flags on attachment: 168270 Committed r131099: <http://trac.webkit.org/changeset/131099>
All reviewed patches have been landed. Closing bug.