Bug 22402 - Replace abort() with CRASH()
Summary: Replace abort() with CRASH()
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Template Framework (show other bugs)
Version: 528+ (Nightly build)
Hardware: Macintosh OS X 10.5
: P2 Normal
Assignee: Alexey Proskuryakov
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2008-11-21 07:12 PST by Alexey Proskuryakov
Modified: 2008-11-21 13:20 PST (History)
0 users

See Also:


Attachments
proposed patch (8.03 KB, patch)
2008-11-21 07:13 PST, Alexey Proskuryakov
no flags Details | Formatted Diff | Diff
abort() if crashing failed (8.90 KB, patch)
2008-11-21 10:42 PST, Alexey Proskuryakov
no flags Details | Formatted Diff | Diff
take three (8.92 KB, patch)
2008-11-21 12:45 PST, Alexey Proskuryakov
mitz: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Alexey Proskuryakov 2008-11-21 07:12:34 PST
Using abort() for fatal failures is bad for at least two reasons:
1) We don't get a crash log.
2) It's impolite to silently close an application on failure, as if it was never running.

We already use CRASH everywhere, except for FastMalloc and one place in Collector.

We could consider taking measures against using abort (or just define it to CRASH), but I'm not sure how much that would help in practice.
Comment 1 Alexey Proskuryakov 2008-11-21 07:13:45 PST
Created attachment 25352 [details]
proposed patch
Comment 2 Sam Weinig 2008-11-21 10:09:28 PST
Comment on attachment 25352 [details]
proposed patch

r=me
Comment 3 Alexey Proskuryakov 2008-11-21 10:42:51 PST
Created attachment 25358 [details]
abort() if crashing failed

Mitz reminds that CRASH() is not 100% reliable. Added an abort() call after it.
Comment 4 Alexey Proskuryakov 2008-11-21 10:49:29 PST
Committed revision 38665.

Comment 5 Alexey Proskuryakov 2008-11-21 12:10:44 PST
Reverted in r38672 - this was somehow breaking this code in Interpreter.cpp:
            #define ADD_OPCODE_ID(id) m_opcodeIDTable.add(&&id, id);
                FOR_EACH_OPCODE_ID(ADD_OPCODE_ID);
            #undef ADD_OPCODE_ID

Comment 6 Alexey Proskuryakov 2008-11-21 12:11:19 PST
Comment on attachment 25358 [details]
abort() if crashing failed

Clearing review flag.
Comment 7 Alexey Proskuryakov 2008-11-21 12:45:23 PST
Created attachment 25360 [details]
take three
Comment 8 Alexey Proskuryakov 2008-11-21 13:20:40 PST
Committed revision 38673.