Bug 7080

Summary: Provide some way to stop a JavaScript infinite loop
Product: WebKit Reporter: Joost de Valk (AlthA) <joost>
Component: JavaScriptCoreAssignee: Anders Carlsson <andersca>
Status: RESOLVED FIXED    
Severity: Critical CC: active.23.13, bugzilla, darin, gavin.sharp, jay, krishnamurty.podipireddy
Priority: P1 Keywords: InRadar
Version: 420+   
Hardware: Mac   
OS: OS X 10.4   
Bug Depends on:    
Bug Blocks: 6958, 3552, 6693    
Attachments:
Description Flags
Hang detection
none
Much better patch
mjs: review+
Add WebKit delegate
none
Address comments
none
Address Geoff's comments ggaren: review+

Joost de Valk (AlthA)
Reported 2006-02-05 01:16:56 PST
At current, infinite javascript loops are really infinite, we need a way to detect and stop them. This currently occurs in: - bug 3552 - bug 6693 - bug 6958 This bug is here to track the development of a way to get out of this loop. All three bugs above have testcases, which could be used to test this behavior until their underlying bugs are fixed. All three of them have a root cause getting them into this infinite loop, so if these root causes are fixed they won't be useful for testing anymore.
Attachments
Hang detection (9.04 KB, patch)
2006-06-15 16:28 PDT, Anders Carlsson 		no flags
DetailsFormatted DiffDiff
Much better patch (12.54 KB, patch)
2006-06-16 16:08 PDT, Anders Carlsson 		mjs: review+
DetailsFormatted DiffDiff
Add WebKit delegate (13.36 KB, patch)
2006-06-17 08:28 PDT, Anders Carlsson 		no flags
DetailsFormatted DiffDiff
Address comments (13.14 KB, patch)
2006-06-18 09:42 PDT, Anders Carlsson 		no flags
DetailsFormatted DiffDiff
Address Geoff's comments (14.13 KB, patch)
2006-06-18 09:59 PDT, Anders Carlsson 		ggaren: review+
DetailsFormatted DiffDiff
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.
Geoffrey Garen
Comment 1 2006-02-06 09:13:54 PST
In Radar as <rdar://problem/3895579>
Alexey Proskuryakov
Comment 2 2006-02-07 13:30:36 PST
*** Bug 6089 has been marked as a duplicate of this bug. ***
Krishna
Comment 3 2006-05-16 17:03:07 PDT
*** Bug 8949 has been marked as a duplicate of this bug. ***
Anders Carlsson
Comment 4 2006-06-15 16:28:38 PDT
Created attachment 8863 [details] Hang detection This implements the JSC part of a hang-detection implementation that works pretty much like the one in Mozilla. Unfortunately, when turned on this causes a performance regression in ibench. The other approach I tried (using UNIX signals) proved to be complicated and error-prone and it too gave a performance regression (albeit a smaller one)
Darin Adler
Comment 5 2006-06-15 16:32:09 PDT
Off. It's going to be tricky to come up with a way to do this without slowing things down!
Geoffrey Garen
Comment 6 2006-06-15 19:43:53 PDT
Comment on attachment 8863 [details] Hang detection Clearing the review bit, since this was a perf. regression.
Anders Carlsson
Comment 7 2006-06-16 16:08:28 PDT
Created attachment 8872 [details] Much better patch Here's a new patch that doesn't cause any measurable slowdown at all (Less than 0.2% on ibench)
Maciej Stachowiak
Comment 8 2006-06-16 16:31:06 PDT
Comment on attachment 8872 [details] Much better patch r=me ggaren likes it too
David Kilzer (:ddkilzer)
Comment 9 2006-06-16 18:20:46 PDT
Committed as r14893.
Anders Carlsson
Comment 10 2006-06-17 08:28:11 PDT
Created attachment 8886 [details] Add WebKit delegate
Darin Adler
Comment 11 2006-06-17 17:16:44 PDT
Comment on attachment 8886 [details] Add WebKit delegate SCRIPT_TIMEOUT_TIME_MS should be a C++ const, not a macro. The shouldInterruptScript functions could be const. The WebDefaultUIDelegate is a pretty heavyweight way to make shouldInterruptJavaScript default to no. WebFrameBridge should just say return NO -- I don't think we need this method int the WebDefaultUIDelegate.
Anders Carlsson
Comment 12 2006-06-18 09:42:32 PDT
Created attachment 8902 [details] Address comments
Anders Carlsson
Comment 13 2006-06-18 09:59:59 PDT
Created attachment 8903 [details] Address Geoff's comments I talked to Geoff on IRC and he had a couple of comments which this patch addresses.
Geoffrey Garen
Comment 14 2006-06-18 10:02:16 PDT
Comment on attachment 8903 [details] Address Geoff's comments r=me and the crowd goes wild
Note You need to log in before you can comment on or make changes to this bug.