Bug 37797
| Summary: | Add a flag to make DRT use exit(1) to avoid ReportCrash starting | ||
|---|---|---|---|
| Product: | WebKit | Reporter: | Ojan Vafai <ojan> |
| Component: | Tools / Tests | Assignee: | Nobody <webkit-unassigned> |
| Status: | NEW | ||
| Severity: | Normal | CC: | ap, aroben, eric, mjs, sam |
| Priority: | P2 | ||
| Version: | 528+ (Nightly build) | ||
| Hardware: | PC | ||
| OS: | OS X 10.5 | ||
| Bug Depends on: | |||
| Bug Blocks: | 64491 | ||
Ojan Vafai
From bug 37738:
> > > Also, is there a way we can kill ReportCrash from NRWT? When I run the tests
> > > locally, I frequently will "sudo killall ReportCrash" in a loop. I'd much
> > > rather have NRWT do that for me. The sudo is the problem of course.
> >
> > I don't think that would be a good thing to do. The crash logs are very useful
> > for diagnosing what went wrong, especially on the buildbots.
>
> I've long wanted such an option to run-webkit-tests. It's easy enough to do.
> Just requires adding a flag to DumpRenderTree to have it catch the necessary
> signals and exit(1) instead of letting the OS catch them for us and calling
> ReportCrash. I don't think we'd want this mode on by default, but it would be
> useful.
I think we should make it the default and have a flag to turn it off on the bots. Also, we do this with TestShell on the Mac. It reduces the number of cases where ReportCrash fires up, but doesn't seem to catch all of them.
| Attachments | ||
|---|---|---|
| Add attachment proposed patch, testcase, etc. |
Sam Weinig
To fully avoid ReportCrash, you can install signal handlers. We do this for the jsc executable, see http://trac.webkit.org/browser/trunk/JavaScriptCore/jsc.cpp#L498.
Mark Rowe (bdash)
Can someone clarify why this should be the default? If I make a code change, run the tests, and it causes a crash or assertion failure I want to know where it crashed or asserted. In what cases would you not want to know where or why it crashed?
Ojan Vafai
(In reply to comment #2)
> Can someone clarify why this should be the default? If I make a code change,
> run the tests, and it causes a crash or assertion failure I want to know where
> it crashed or asserted. In what cases would you not want to know where or why
> it crashed?
Maybe I'm the exception, but when I hit a crash or assert running tests, I typically will run that specific test directly from xcode rather than looking at the crash log. The only exception to this is if the crash doesn't happen reliably, in which case I'd be ok with turning crash reporting on.
Making this the default makes hacking on webkit more inviting to new or occasional contributors. People who don't regularly hack on Macs and/or WebKit-Mac, don't remember that ReportCrash exists and that it considerably slows down running the tests.
Also, it's not uncommon for there to be crashes checked in to trunk. I don't want run-webkit-tests to take forever because someone else's patch is causing a handful of crashes.
Alexey Proskuryakov
I'd also prefer CrashReporter run by default. Not having crash logs means loss of information - you can wait a moment for CrashReporter to finish, but you can't get a crash log for an unreproducible crash if it wasn't ever generated.
Ojan Vafai
I don't feel strongly about the default value. The flag should be there though.
Eric Seidel (no email)
This would be super useful. I've hacked this into DRT in the past when debugging crashers. I also don't care about the default, but assume it should be off by default, and folks who wish to use it to debug crashers can pass the flag.