Bug 150103 - [Win] Generate Crash Traces
Summary: [Win] Generate Crash Traces
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Brent Fulgham
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-10-13 15:34 PDT by Brent Fulgham
Modified: 2015-10-13 17:16 PDT (History)
6 users (show)

See Also:


Attachments
Patch (6.12 KB, patch)
2015-10-13 15:43 PDT, Brent Fulgham
dbates: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Brent Fulgham 2015-10-13 15:34:21 PDT
The test system has not been reliably generating crash traces on Windows. This seems to have started some time ago, and seems to be related to some bugs in the Python code as well as changes in the way MSVC deals with termination handlers.

This patch updates the test infrastructure so that we get crash traces more reliably.
Comment 1 Brent Fulgham 2015-10-13 15:43:36 PDT
Created attachment 263030 [details]
Patch
Comment 2 Daniel Bates 2015-10-13 16:43:45 PDT
Comment on attachment 263030 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=263030&action=review

> Tools/ChangeLog:19
> +        (CrashLogs): Add another REGEX to handle a second crash trace syntax I

Nit: REGEX => regex or regular expression

> Tools/ChangeLog:21
> +        (CrashLogs._find_newest_log_win): If the old REGEX doesn't match, try

Ditto.

> Tools/ChangeLog:22
> +        the new one. The new REGEX finds a PID in HEX, so convert to integer.

Nit: REGEX => regex or regular expression
Nit: "in HEX" => "given as hexadecimal number"

> Tools/Scripts/webkitpy/common/system/crashlogs.py:37
> +    PID_LINE_REGEX2 = re.compile(r'Exit process \d+:(?P<pid>\w+), code')

This is OK as-is. Maybe a better name for PID_LINE_REGEX2 would be EXIT_PROCESS_PID_REGEX? We may also want to consider coming up with a better name for PID_LINE_REGEX. Maybe GLOBAL_PID_REGEX?

We may want to consider strengthening this regular expression to only accept ASCII hex digits for the PID instead of \w+.
Comment 3 Brent Fulgham 2015-10-13 16:51:37 PDT
Comment on attachment 263030 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=263030&action=review

>> Tools/ChangeLog:19
>> +        (CrashLogs): Add another REGEX to handle a second crash trace syntax I
> 
> Nit: REGEX => regex or regular expression

Will fix.

>> Tools/ChangeLog:21
>> +        (CrashLogs._find_newest_log_win): If the old REGEX doesn't match, try
> 
> Ditto.

Will fix.

>> Tools/ChangeLog:22
>> +        the new one. The new REGEX finds a PID in HEX, so convert to integer.
> 
> Nit: REGEX => regex or regular expression
> Nit: "in HEX" => "given as hexadecimal number"

Will fix.

>> Tools/Scripts/webkitpy/common/system/crashlogs.py:37
>> +    PID_LINE_REGEX2 = re.compile(r'Exit process \d+:(?P<pid>\w+), code')
> 
> This is OK as-is. Maybe a better name for PID_LINE_REGEX2 would be EXIT_PROCESS_PID_REGEX? We may also want to consider coming up with a better name for PID_LINE_REGEX. Maybe GLOBAL_PID_REGEX?
> 
> We may want to consider strengthening this regular expression to only accept ASCII hex digits for the PID instead of \w+.

I agree about the name changes, and will do so. I'll use GLOBAL_PID_REGEX and EXIT_PROCESS_PID_REGEX.
Comment 4 Brent Fulgham 2015-10-13 17:16:10 PDT
Committed r191018: <http://trac.webkit.org/changeset/191018>