178402 2017-10-17 11:21:07 -0700 REGRESSION (r222709): webkitpy: Hang when workers write to the same stack trace file 2017-11-29 10:16:12 -0800 1 1 1 Unclassified WebKit Tools / Tests WebKit Nightly Build Unspecified Unspecified RESOLVED CONFIGURATION CHANGED https://bugs.webkit.org/show_bug.cgi?id=178368 https://bugs.webkit.org/show_bug.cgi?id=176393 InRadar, Regression P2 Normal --- 176393 178587 1 jbedard jbedard aakash_jain ap buildbot commit-queue dbates glenn lforschler webkit-bug-importer oldest_to_newest 1361322 0 jbedard 2017-10-17 11:21:07 -0700 There is a hang when workers write their stack traces to the same file if that file already exists on disk. This is the root cause of the more general issue addressed in <https://bugs.webkit.org/show_bug.cgi?id=178368>. 1361325 1 webkit-bug-importer 2017-10-17 11:21:41 -0700 <rdar://problem/35033432> 1361328 2 jbedard 2017-10-17 11:22:06 -0700 If we add a pid to the name of the stack-trace, the problem stops. 1361341 3 324035 jbedard 2017-10-17 11:45:32 -0700 Created attachment 324035 Patch 1361354 4 324035 aakash_jain 2017-10-17 12:14:12 -0700 Comment on attachment 324035 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=324035&action=review > Tools/Scripts/webkitpy/common/interupt_debugging.py:47 > + file_name = os.path.join(os.path.dirname(output_file), '{}-{}'.format(os.getpid(), os.path.basename(output_file))) if output_file else None Is it guaranteed that we won't have file name collision after adding os.getpid() ? Maybe we can add timestamp in filename instead of pid to make it unique, it would also make it easy to sort these files into the directory having multiple such files. 1361375 5 jbedard 2017-10-17 13:02:47 -0700 (In reply to Aakash Jain from comment #4) > Comment on attachment 324035 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=324035&action=review > > > Tools/Scripts/webkitpy/common/interupt_debugging.py:47 > > + file_name = os.path.join(os.path.dirname(output_file), '{}-{}'.format(os.getpid(), os.path.basename(output_file))) if output_file else None > > Is it guaranteed that we won't have file name collision after adding > os.getpid() ? > Maybe we can add timestamp in filename instead of pid to make it unique, it > would also make it easy to sort these files into the directory having > multiple such files. It's guaranteed that we won't have a file name collision inside a single test run. Overwriting an existing file with the same name is perfectly fine. I don't think timestamp is a good way to achieve this because it is possible (likely, in fact) that workers will generate timestamps very close to or even equal to their sibling workers since they will all receive the SIGTERM at the same time. 1361387 6 324035 commit-queue 2017-10-17 13:30:21 -0700 Comment on attachment 324035 Patch Clearing flags on attachment: 324035 Committed r223572: <https://trac.webkit.org/changeset/223572> 1361388 7 commit-queue 2017-10-17 13:30:23 -0700 All reviewed patches have been landed. Closing bug. 1362708 8 commit-queue 2017-10-20 09:06:59 -0700 Re-opened since this is blocked by bug 178587 1362718 9 ap 2017-10-20 09:14:28 -0700 This didn't help, at least not enough. Re-fixed by rolling back both r222709 and r223572. 1375887 10 jbedard 2017-11-29 10:16:12 -0800 Note that this change is now part of the patch up for review in <https://bugs.webkit.org/show_bug.cgi?id=176393>. 324035 2017-10-17 11:45:32 -0700 2017-10-17 13:30:21 -0700 Patch bug-178402-20171017114531.patch text/plain 3368 jbedard SW5kZXg6IFRvb2xzL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBUb29scy9DaGFuZ2VMb2cJKHJl dmlzaW9uIDIyMzU1OCkKKysrIFRvb2xzL0NoYW5nZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwz ICsxLDE5IEBACisyMDE3LTEwLTE3ICBKb25hdGhhbiBCZWRhcmQgIDxqYmVkYXJkQGFwcGxlLmNv bT4KKworICAgICAgICB3ZWJraXRweTogSGFuZyB3aGVuIHdvcmtlcnMgd3JpdGUgdG8gdGhlIHNh bWUgc3RhY2sgdHJhY2UgZmlsZQorICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93 X2J1Zy5jZ2k/aWQ9MTc4NDAyCisgICAgICAgIDxyZGFyOi8vcHJvYmxlbS8zNTAzMzQzMj4KKwor ICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICBXb3JrZXJzIGNh biBoYW5nIGlmIHRoZXkgYWxsIHdyaXRlIHRvIHRoZSBzYW1lIHN0YWNrIHRyYWNlIGZpbGUgd2hl biByZWNlaXZpbmcKKyAgICAgICAgYSBTSUdURVJNLiBBdHRhY2ggdGhlIHBpZCB0byB0aGUgc3Rh Y2sgdHJhY2UgZmlsZSBuYW1lIHNvIHRoYXQgZWFjaCB3b3JrZXIKKyAgICAgICAgd3JpdGVzIHRv IGEgZGlmZmVyZW50IGZpbGUuCisKKyAgICAgICAgKiBTY3JpcHRzL3dlYmtpdHB5L2NvbW1vbi9p bnRlcnVwdF9kZWJ1Z2dpbmcucHk6CisgICAgICAgIChsb2dfc3RhY2tfdHJhY2Vfb25fdGVybS5o YW5kbGVyKTogTmFtZSBzdGFjayB0cmFjZSBmaWxlIHBhdGgvPHBpZD4tZmlsZW5hbWUuCisgICAg ICAgIChsb2dfc3RhY2tfdHJhY2Vfb25fY250cmxfYy5oYW5kbGVyKTogRGl0dG8uCisKIDIwMTct MTAtMTcgIFlvdWVubiBGYWJsZXQgIDx5b3Vlbm5AYXBwbGUuY29tPgogCiAgICAgICAgIENhY2hl IEFQSSBpbXBsZW1lbnRhdGlvbiBzaG91bGQgYmUgYWJsZSB0byBjb21wdXRlIHN0b3JhZ2Ugc2l6 ZSBmb3IgV2ViS2l0IGNsaWVudCBhcHBsaWNhdGlvbnMuCkluZGV4OiBUb29scy9TY3JpcHRzL3dl YmtpdHB5L2NvbW1vbi9pbnRlcnVwdF9kZWJ1Z2dpbmcucHkKPT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gVG9vbHMv U2NyaXB0cy93ZWJraXRweS9jb21tb24vaW50ZXJ1cHRfZGVidWdnaW5nLnB5CShyZXZpc2lvbiAy MjM0MjYpCisrKyBUb29scy9TY3JpcHRzL3dlYmtpdHB5L2NvbW1vbi9pbnRlcnVwdF9kZWJ1Z2dp bmcucHkJKHdvcmtpbmcgY29weSkKQEAgLTIwLDYgKzIwLDcgQEAKICMgT1IgVE9SVCAoSU5DTFVE SU5HIE5FR0xJR0VOQ0UgT1IgT1RIRVJXSVNFKSBBUklTSU5HIElOIEFOWSBXQVkgT1VUIE9GIFRI RSBVU0UKICMgT0YgVEhJUyBTT0ZUV0FSRSwgRVZFTiBJRiBBRFZJU0VEIE9GIFRIRSBQT1NTSUJJ TElUWSBPRiBTVUNIIERBTUFHRS4KIAoraW1wb3J0IG9zCiBpbXBvcnQgbGluZWNhY2hlCiBpbXBv cnQgbG9nZ2luZwogaW1wb3J0IHNpZ25hbApAQCAtNDMsMTQgKzQ0LDE1IEBAIGRlZiBsb2dfc3Rh Y2tfdHJhY2UoZnJhbWUsIGZpbGUpOgogCiBkZWYgbG9nX3N0YWNrX3RyYWNlX29uX3Rlcm0ob3V0 cHV0X2ZpbGU9Tm9uZSk6CiAgICAgZGVmIGhhbmRsZXIoc2lnbnVtLCBmcmFtZSk6Ci0gICAgICAg IGZpbGUgPSBvcGVuKG91dHB1dF9maWxlLCAndycpIGlmIG91dHB1dF9maWxlIGVsc2Ugc3lzLnN0 ZGVycgorICAgICAgICBmaWxlX25hbWUgPSBvcy5wYXRoLmpvaW4ob3MucGF0aC5kaXJuYW1lKG91 dHB1dF9maWxlKSwgJ3t9LXt9Jy5mb3JtYXQob3MuZ2V0cGlkKCksIG9zLnBhdGguYmFzZW5hbWUo b3V0cHV0X2ZpbGUpKSkgaWYgb3V0cHV0X2ZpbGUgZWxzZSBOb25lCisgICAgICAgIGZpbGUgPSBv cGVuKGZpbGVfbmFtZSwgJ3cnKSBpZiBmaWxlX25hbWUgZWxzZSBzeXMuc3RkZXJyCiAgICAgICAg IGlmIG5vdCBmaWxlOgotICAgICAgICAgICAgcmFpc2UgUnVudGltZUVycm9yKCd7fSBjYW5ub3Qg YmUgb3BlbmVkJy5mb3JtYXQob3V0cHV0X2ZpbGUpKQorICAgICAgICAgICAgcmFpc2UgUnVudGlt ZUVycm9yKCd7fSBjYW5ub3QgYmUgb3BlbmVkJy5mb3JtYXQoZmlsZV9uYW1lKSkKIAogICAgICAg ICBpZiBmaWxlIGlzIHN5cy5zdGRlcnI6CiAgICAgICAgICAgICBmaWxlLndyaXRlKCdcbicpCiAg ICAgICAgIGVsc2U6Ci0gICAgICAgICAgICBfbG9nLmNyaXRpY2FsKCdTdGFjayB0cmFjZSBzYXZl ZCB0byB7fScuZm9ybWF0KG91dHB1dF9maWxlKSkKKyAgICAgICAgICAgIF9sb2cuY3JpdGljYWwo J1N0YWNrIHRyYWNlIHNhdmVkIHRvIHt9Jy5mb3JtYXQoZmlsZV9uYW1lKSkKICAgICAgICAgZmls ZS53cml0ZSgnU0lHVEVSTSBzaWduYWwgcmVjZWl2ZWQnKQogICAgICAgICBsb2dfc3RhY2tfdHJh Y2UoZnJhbWUsIGZpbGUpCiAgICAgICAgIGV4aXQoLTEpCkBAIC02MCwxNCArNjIsMTUgQEAgZGVm IGxvZ19zdGFja190cmFjZV9vbl90ZXJtKG91dHB1dF9maWxlPQogCiBkZWYgbG9nX3N0YWNrX3Ry YWNlX29uX2NudHJsX2Mob3V0cHV0X2ZpbGU9Tm9uZSk6CiAgICAgZGVmIGhhbmRsZXIoc2lnbnVt LCBmcmFtZSk6Ci0gICAgICAgIGZpbGUgPSBvcGVuKG91dHB1dF9maWxlLCAndycpIGlmIG91dHB1 dF9maWxlIGVsc2Ugc3lzLnN0ZGVycgorICAgICAgICBmaWxlX25hbWUgPSBvcy5wYXRoLmpvaW4o b3MucGF0aC5kaXJuYW1lKG91dHB1dF9maWxlKSwgJ3t9LXt9Jy5mb3JtYXQob3MuZ2V0cGlkKCks IG9zLnBhdGguYmFzZW5hbWUob3V0cHV0X2ZpbGUpKSkgaWYgb3V0cHV0X2ZpbGUgZWxzZSBOb25l CisgICAgICAgIGZpbGUgPSBvcGVuKGZpbGVfbmFtZSwgJ3cnKSBpZiBmaWxlX25hbWUgZWxzZSBz eXMuc3RkZXJyCiAgICAgICAgIGlmIG5vdCBmaWxlOgotICAgICAgICAgICAgcmFpc2UgUnVudGlt ZUVycm9yKCd7fSBjYW5ub3QgYmUgb3BlbmVkJy5mb3JtYXQob3V0cHV0X2ZpbGUpKQorICAgICAg ICAgICAgcmFpc2UgUnVudGltZUVycm9yKCd7fSBjYW5ub3QgYmUgb3BlbmVkJy5mb3JtYXQoZmls ZV9uYW1lKSkKIAogICAgICAgICBpZiBmaWxlIGlzIHN5cy5zdGRlcnI6CiAgICAgICAgICAgICBm aWxlLndyaXRlKCdcbicpCiAgICAgICAgIGVsc2U6Ci0gICAgICAgICAgICBfbG9nLmNyaXRpY2Fs KCdTdGFjayB0cmFjZSBzYXZlZCB0byB7fScuZm9ybWF0KG91dHB1dF9maWxlKSkKKyAgICAgICAg ICAgIF9sb2cuY3JpdGljYWwoJ1N0YWNrIHRyYWNlIHNhdmVkIHRvIHt9Jy5mb3JtYXQoZmlsZV9u YW1lKSkKICAgICAgICAgZmlsZS53cml0ZSgnY250cmwgQyByZWNlaXZlZFxuJykKICAgICAgICAg bG9nX3N0YWNrX3RyYWNlKGZyYW1lLCBmaWxlKQogICAgICAgICByYWlzZSBLZXlib2FyZEludGVy cnVwdAo=