Bug 178402

Summary: REGRESSION (r222709): webkitpy: Hang when workers write to the same stack trace file
Product: WebKit Reporter: Jonathan Bedard <jbedard>
Component: Tools / TestsAssignee: Jonathan Bedard <jbedard>
Status: RESOLVED CONFIGURATION CHANGED    
Severity: Normal CC: aakash_jain, ap, buildbot, commit-queue, dbates, glenn, lforschler, webkit-bug-importer
Priority: P2 Keywords: InRadar, Regression
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=178368
https://bugs.webkit.org/show_bug.cgi?id=176393
Bug Depends on: 176393, 178587    
Bug Blocks:    
Attachments:
Description Flags
Patch none

Jonathan Bedard
Reported 2017-10-17 11:21:07 PDT
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>.
Attachments
Patch (3.29 KB, patch)
2017-10-17 11:45 PDT, Jonathan Bedard
no flags
Radar WebKit Bug Importer
Comment 1 2017-10-17 11:21:41 PDT
Jonathan Bedard
Comment 2 2017-10-17 11:22:06 PDT
If we add a pid to the name of the stack-trace, the problem stops.
Jonathan Bedard
Comment 3 2017-10-17 11:45:32 PDT
Aakash Jain
Comment 4 2017-10-17 12:14:12 PDT
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.
Jonathan Bedard
Comment 5 2017-10-17 13:02:47 PDT
(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.
WebKit Commit Bot
Comment 6 2017-10-17 13:30:21 PDT
Comment on attachment 324035 [details] Patch Clearing flags on attachment: 324035 Committed r223572: <https://trac.webkit.org/changeset/223572>
WebKit Commit Bot
Comment 7 2017-10-17 13:30:23 PDT
All reviewed patches have been landed. Closing bug.
WebKit Commit Bot
Comment 8 2017-10-20 09:06:59 PDT
Re-opened since this is blocked by bug 178587
Alexey Proskuryakov
Comment 9 2017-10-20 09:14:28 PDT
This didn't help, at least not enough. Re-fixed by rolling back both r222709 and r223572.
Jonathan Bedard
Comment 10 2017-11-29 10:16:12 PST
Note that this change is now part of the patch up for review in <https://bugs.webkit.org/show_bug.cgi?id=176393>.
Note You need to log in before you can comment on or make changes to this bug.