99084 2012-10-11 10:31:33 -0700 [Chromium-Android] Exception when the layout test driver is stopped 2012-10-11 15:25:54 -0700 1 1 1 Unclassified WebKit Tools / Tests 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED NRWT P2 Normal --- 1 wangxianzhu wangxianzhu abarth dpranke ojan peter webkit.review.bot oldest_to_newest 740116 0 wangxianzhu 2012-10-11 10:31:33 -0700 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 740130 1 168254 wangxianzhu 2012-10-11 10:47:10 -0700 Created attachment 168254 Patch 740180 2 168254 dpranke 2012-10-11 12:48:56 -0700 Comment on attachment 168254 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). 740201 3 wangxianzhu 2012-10-11 13:18:36 -0700 (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? 740213 4 dpranke 2012-10-11 13:28:59 -0700 (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? 740226 5 wangxianzhu 2012-10-11 13:39:17 -0700 (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 :) 740231 6 168270 wangxianzhu 2012-10-11 13:41:40 -0700 Created attachment 168270 Patch 740303 7 168270 webkit.review.bot 2012-10-11 15:25:51 -0700 Comment on attachment 168270 Patch Clearing flags on attachment: 168270 Committed r131099: <http://trac.webkit.org/changeset/131099> 740304 8 webkit.review.bot 2012-10-11 15:25:54 -0700 All reviewed patches have been landed. Closing bug. 168254 2012-10-11 10:47:10 -0700 2012-10-11 13:41:37 -0700 Patch bug-99084-20121011104607.patch text/plain 2551 wangxianzhu U3VidmVyc2lvbiBSZXZpc2lvbjogMTMxMDc5CmRpZmYgLS1naXQgYS9Ub29scy9DaGFuZ2VMb2cg Yi9Ub29scy9DaGFuZ2VMb2cKaW5kZXggZDg4OTljZDZmMjkzOWY4MjIzYzIwMTkwZWJkZGMwNGE4 NTNjOWIxYS4uNjI5NWVlYmMxNjliYmEzNmUzY2I1MDEwODUzYzlkMzE5Y2EzOTJlMyAxMDA2NDQK LS0tIGEvVG9vbHMvQ2hhbmdlTG9nCisrKyBiL1Rvb2xzL0NoYW5nZUxvZwpAQCAtMSwzICsxLDE2 IEBACisyMDEyLTEwLTExICBYaWFuemh1IFdhbmcgIDx3YW5neGlhbnpodUBjaHJvbWl1bS5vcmc+ CisKKyAgICAgICAgW0Nocm9taXVtLUFuZHJvaWRdIEV4Y2VwdGlvbiB3aGVuIHRoZSBsYXlvdXQg dGVzdCBkcml2ZXIgaXMgc3RvcHBlZAorICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9z aG93X2J1Zy5jZ2k/aWQ9OTkwODQKKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMh KS4KKworICAgICAgICBPbiBjaHJvbWl1bS1hbmRyb2lkLCB0aGUgcHJvY2VzcyBpcyBraWxsZWQg ZGlyZWN0bHkgaW4gU2VydmVyUHJvY2Vzcy5zdG9wKCkgYXMgdGhlIHBpcGVzIGFyZSBjbG9zZWQg Zmlyc3QuCisgICAgICAgIFNob3VsZCBub3QgdHJ5IHRvIHJlYWQgZGF0YSBmcm9tIHRoZSBwaXBl cyBhZnRlciB0aGUgcHJvY2VzcyBpcyBraWxsZWQuCisKKyAgICAgICAgKiBTY3JpcHRzL3dlYmtp dHB5L2xheW91dF90ZXN0cy9wb3J0L3NlcnZlcl9wcm9jZXNzLnB5OgorICAgICAgICAoU2VydmVy UHJvY2Vzcy5zdG9wKTogRG9uJ3QgcmVhZCBkYXRhIGFmdGVyIHRoZSBwcm9jZXNzIGlzIGtpbGxl ZC4KKwogMjAxMi0xMC0xMSAgTWFyaW8gU2FuY2hleiBQcmFkYSAgPG1zYW5jaGV6QGlnYWxpYS5j b20+CiAKICAgICAgICAgW0dUS10gUkVHUkVTU0lPTihyMTMxMDMzKTogRmF2aWNvbnMgZG9uJ3Qg d29yayBpbiBNaW5pQnJvd3NlcgpkaWZmIC0tZ2l0IGEvVG9vbHMvU2NyaXB0cy93ZWJraXRweS9s YXlvdXRfdGVzdHMvcG9ydC9zZXJ2ZXJfcHJvY2Vzcy5weSBiL1Rvb2xzL1NjcmlwdHMvd2Via2l0 cHkvbGF5b3V0X3Rlc3RzL3BvcnQvc2VydmVyX3Byb2Nlc3MucHkKaW5kZXggNDhmZWRhOTY4NmE4 YWE0NjUxYzE5NWFkMGRlZDFlNzRlZjdhN2ZkMi4uYjI1ZjljOTZlZDEwODcxMzU1NTExYzc0NTQw MjlmZDE0MWZhNzA3OSAxMDA2NDQKLS0tIGEvVG9vbHMvU2NyaXB0cy93ZWJraXRweS9sYXlvdXRf dGVzdHMvcG9ydC9zZXJ2ZXJfcHJvY2Vzcy5weQorKysgYi9Ub29scy9TY3JpcHRzL3dlYmtpdHB5 L2xheW91dF90ZXN0cy9wb3J0L3NlcnZlcl9wcm9jZXNzLnB5CkBAIC0zMzIsOCArMzMyLDEwIEBA IGNsYXNzIFNlcnZlclByb2Nlc3Mob2JqZWN0KToKIAogICAgICAgICBub3cgPSB0aW1lLnRpbWUo KQogICAgICAgICBzZWxmLl9wcm9jLnN0ZGluLmNsb3NlKCkKKyAgICAgICAga2lsbGVkID0gRmFs c2UKICAgICAgICAgaWYgbm90IHRpbWVvdXRfc2VjczoKICAgICAgICAgICAgIHNlbGYuX2tpbGwo KQorICAgICAgICAgICAga2lsbGVkID0gVHJ1ZQogICAgICAgICBlbGlmIG5vdCBzZWxmLl9ob3N0 LnBsYXRmb3JtLmlzX3dpbigpOgogICAgICAgICAgICAgIyBGSVhNRTogV2h5IGFyZW4ndCB3ZSBj YWxsaW5nIHRoaXMgb24gd2luPwogICAgICAgICAgICAgZGVhZGxpbmUgPSBub3cgKyB0aW1lb3V0 X3NlY3MKQEAgLTM0MiwxMyArMzQ0LDE1IEBAIGNsYXNzIFNlcnZlclByb2Nlc3Mob2JqZWN0KToK ICAgICAgICAgICAgIGlmIHNlbGYuX3Byb2MucG9sbCgpIGlzIE5vbmU6CiAgICAgICAgICAgICAg ICAgX2xvZy53YXJuaW5nKCdzdG9wcGluZyAlcyB0aW1lZCBvdXQsIGtpbGxpbmcgaXQnICUgc2Vs Zi5fbmFtZSkKICAgICAgICAgICAgICAgICBzZWxmLl9raWxsKCkKKyAgICAgICAgICAgICAgICBr aWxsZWQgPSBUcnVlCiAgICAgICAgICAgICAgICAgX2xvZy53YXJuaW5nKCdraWxsZWQnKQogCiAg ICAgICAgICMgcmVhZCBhbnkgcmVtYWluaW5nIGRhdGEgb24gdGhlIHBpcGVzIGFuZCByZXR1cm4g aXQuCi0gICAgICAgIGlmIHNlbGYuX3VzZV93aW4zMl9hcGlzOgotICAgICAgICAgICAgc2VsZi5f d2FpdF9mb3JfZGF0YV9hbmRfdXBkYXRlX2J1ZmZlcnNfdXNpbmdfd2luMzJfYXBpcyhub3cpCi0g ICAgICAgIGVsc2U6Ci0gICAgICAgICAgICBzZWxmLl93YWl0X2Zvcl9kYXRhX2FuZF91cGRhdGVf YnVmZmVyc191c2luZ19zZWxlY3Qobm93LCBzdG9wcGluZz1UcnVlKQorICAgICAgICBpZiBub3Qg a2lsbGVkOgorICAgICAgICAgICAgaWYgc2VsZi5fdXNlX3dpbjMyX2FwaXM6CisgICAgICAgICAg ICAgICAgc2VsZi5fd2FpdF9mb3JfZGF0YV9hbmRfdXBkYXRlX2J1ZmZlcnNfdXNpbmdfd2luMzJf YXBpcyhub3cpCisgICAgICAgICAgICBlbHNlOgorICAgICAgICAgICAgICAgIHNlbGYuX3dhaXRf Zm9yX2RhdGFfYW5kX3VwZGF0ZV9idWZmZXJzX3VzaW5nX3NlbGVjdChub3csIHN0b3BwaW5nPVRy dWUpCiAgICAgICAgIG91dCwgZXJyID0gc2VsZi5fb3V0cHV0LCBzZWxmLl9lcnJvcgogICAgICAg ICBzZWxmLl9yZXNldCgpCiAgICAgICAgIHJldHVybiAob3V0LCBlcnIpCg== 168270 2012-10-11 13:41:40 -0700 2012-10-11 15:25:51 -0700 Patch bug-99084-20121011134037.patch text/plain 2581 wangxianzhu U3VidmVyc2lvbiBSZXZpc2lvbjogMTMxMDc5CmRpZmYgLS1naXQgYS9Ub29scy9DaGFuZ2VMb2cg Yi9Ub29scy9DaGFuZ2VMb2cKaW5kZXggZDg4OTljZDZmMjkzOWY4MjIzYzIwMTkwZWJkZGMwNGE4 NTNjOWIxYS4uNjQyZWFhZmM5OTM3MzNlMzgzMmVkZDQ0NDk0Nzc3ZjRiZDdjZDcyYSAxMDA2NDQK LS0tIGEvVG9vbHMvQ2hhbmdlTG9nCisrKyBiL1Rvb2xzL0NoYW5nZUxvZwpAQCAtMSwzICsxLDE2 IEBACisyMDEyLTEwLTExICBYaWFuemh1IFdhbmcgIDx3YW5neGlhbnpodUBjaHJvbWl1bS5vcmc+ CisKKyAgICAgICAgW0Nocm9taXVtLUFuZHJvaWRdIEV4Y2VwdGlvbiB3aGVuIHRoZSBsYXlvdXQg dGVzdCBkcml2ZXIgaXMgc3RvcHBlZAorICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9z aG93X2J1Zy5jZ2k/aWQ9OTkwODQKKworICAgICAgICBSZXZpZXdlZCBieSBEaXJrIFByYW5rZS4K KworICAgICAgICBPbiBjaHJvbWl1bS1hbmRyb2lkLCB0aGUgcHJvY2VzcyBpcyBraWxsZWQgZGly ZWN0bHkgaW4gU2VydmVyUHJvY2Vzcy5zdG9wKCkgYXMgdGhlIHBpcGVzIGFyZSBjbG9zZWQgZmly c3QuCisgICAgICAgIFNob3VsZCBub3QgdHJ5IHRvIHJlYWQgZGF0YSBmcm9tIHRoZSBwaXBlcyBh ZnRlciB0aGUgcHJvY2VzcyBpcyBraWxsZWQuCisKKyAgICAgICAgKiBTY3JpcHRzL3dlYmtpdHB5 L2xheW91dF90ZXN0cy9wb3J0L3NlcnZlcl9wcm9jZXNzLnB5OgorICAgICAgICAoU2VydmVyUHJv Y2Vzcy5zdG9wKTogRG9uJ3QgcmVhZCBkYXRhIGFmdGVyIHRoZSBwcm9jZXNzIGlzIGtpbGxlZC4K KwogMjAxMi0xMC0xMSAgTWFyaW8gU2FuY2hleiBQcmFkYSAgPG1zYW5jaGV6QGlnYWxpYS5jb20+ CiAKICAgICAgICAgW0dUS10gUkVHUkVTU0lPTihyMTMxMDMzKTogRmF2aWNvbnMgZG9uJ3Qgd29y ayBpbiBNaW5pQnJvd3NlcgpkaWZmIC0tZ2l0IGEvVG9vbHMvU2NyaXB0cy93ZWJraXRweS9sYXlv dXRfdGVzdHMvcG9ydC9zZXJ2ZXJfcHJvY2Vzcy5weSBiL1Rvb2xzL1NjcmlwdHMvd2Via2l0cHkv bGF5b3V0X3Rlc3RzL3BvcnQvc2VydmVyX3Byb2Nlc3MucHkKaW5kZXggNDhmZWRhOTY4NmE4YWE0 NjUxYzE5NWFkMGRlZDFlNzRlZjdhN2ZkMi4uYmZkZjgzMDFiYTAwZjkzYjVmYjUyN2MwNDIyNTk4 NjVmOGE3NGUwNSAxMDA2NDQKLS0tIGEvVG9vbHMvU2NyaXB0cy93ZWJraXRweS9sYXlvdXRfdGVz dHMvcG9ydC9zZXJ2ZXJfcHJvY2Vzcy5weQorKysgYi9Ub29scy9TY3JpcHRzL3dlYmtpdHB5L2xh eW91dF90ZXN0cy9wb3J0L3NlcnZlcl9wcm9jZXNzLnB5CkBAIC0zMzIsOCArMzMyLDExIEBAIGNs YXNzIFNlcnZlclByb2Nlc3Mob2JqZWN0KToKIAogICAgICAgICBub3cgPSB0aW1lLnRpbWUoKQog ICAgICAgICBzZWxmLl9wcm9jLnN0ZGluLmNsb3NlKCkKKyAgICAgICAgc2VsZi5fcHJvYy5zdGRp biA9IE5vbmUKKyAgICAgICAga2lsbGVkID0gRmFsc2UKICAgICAgICAgaWYgbm90IHRpbWVvdXRf c2VjczoKICAgICAgICAgICAgIHNlbGYuX2tpbGwoKQorICAgICAgICAgICAga2lsbGVkID0gVHJ1 ZQogICAgICAgICBlbGlmIG5vdCBzZWxmLl9ob3N0LnBsYXRmb3JtLmlzX3dpbigpOgogICAgICAg ICAgICAgIyBGSVhNRTogV2h5IGFyZW4ndCB3ZSBjYWxsaW5nIHRoaXMgb24gd2luPwogICAgICAg ICAgICAgZGVhZGxpbmUgPSBub3cgKyB0aW1lb3V0X3NlY3MKQEAgLTM0MiwxMyArMzQ1LDE1IEBA IGNsYXNzIFNlcnZlclByb2Nlc3Mob2JqZWN0KToKICAgICAgICAgICAgIGlmIHNlbGYuX3Byb2Mu cG9sbCgpIGlzIE5vbmU6CiAgICAgICAgICAgICAgICAgX2xvZy53YXJuaW5nKCdzdG9wcGluZyAl cyB0aW1lZCBvdXQsIGtpbGxpbmcgaXQnICUgc2VsZi5fbmFtZSkKICAgICAgICAgICAgICAgICBz ZWxmLl9raWxsKCkKKyAgICAgICAgICAgICAgICBraWxsZWQgPSBUcnVlCiAgICAgICAgICAgICAg ICAgX2xvZy53YXJuaW5nKCdraWxsZWQnKQogCiAgICAgICAgICMgcmVhZCBhbnkgcmVtYWluaW5n IGRhdGEgb24gdGhlIHBpcGVzIGFuZCByZXR1cm4gaXQuCi0gICAgICAgIGlmIHNlbGYuX3VzZV93 aW4zMl9hcGlzOgotICAgICAgICAgICAgc2VsZi5fd2FpdF9mb3JfZGF0YV9hbmRfdXBkYXRlX2J1 ZmZlcnNfdXNpbmdfd2luMzJfYXBpcyhub3cpCi0gICAgICAgIGVsc2U6Ci0gICAgICAgICAgICBz ZWxmLl93YWl0X2Zvcl9kYXRhX2FuZF91cGRhdGVfYnVmZmVyc191c2luZ19zZWxlY3Qobm93LCBz dG9wcGluZz1UcnVlKQorICAgICAgICBpZiBub3Qga2lsbGVkOgorICAgICAgICAgICAgaWYgc2Vs Zi5fdXNlX3dpbjMyX2FwaXM6CisgICAgICAgICAgICAgICAgc2VsZi5fd2FpdF9mb3JfZGF0YV9h bmRfdXBkYXRlX2J1ZmZlcnNfdXNpbmdfd2luMzJfYXBpcyhub3cpCisgICAgICAgICAgICBlbHNl OgorICAgICAgICAgICAgICAgIHNlbGYuX3dhaXRfZm9yX2RhdGFfYW5kX3VwZGF0ZV9idWZmZXJz X3VzaW5nX3NlbGVjdChub3csIHN0b3BwaW5nPVRydWUpCiAgICAgICAgIG91dCwgZXJyID0gc2Vs Zi5fb3V0cHV0LCBzZWxmLl9lcnJvcgogICAgICAgICBzZWxmLl9yZXNldCgpCiAgICAgICAgIHJl dHVybiAob3V0LCBlcnIpCg==