REGRESSION (r222709): webkitpy: Hang when workers write to the same stack trace file

• Component: Tools / Tests
• Status: RESOLVED CONFIGURATION CHANGED

[Jonathan Bedard]

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>.

[Radar WebKit Bug Importer]

<rdar://problem/35033432>

[Jonathan Bedard]

If we add a pid to the name of the stack-trace, the problem stops.

[Jonathan Bedard]

Created attachment 324035 [details]
Patch

[Aakash Jain]

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]

(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 on attachment 324035 [details]
Patch

Clearing flags on attachment: 324035

Committed r223572: <https://trac.webkit.org/changeset/223572>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[WebKit Commit Bot]

Re-opened since this is blocked by bug 178587

[Alexey Proskuryakov]

This didn't help, at least not enough. Re-fixed by rolling back both r222709 and r223572.

[Jonathan Bedard]

Note that this change is now part of the patch up for review in <https://bugs.webkit.org/show_bug.cgi?id=176393>.