NEW 79040
(function () { while(true) { } })() runs forever with DFG JIT 
https://bugs.webkit.org/show_bug.cgi?id=79040
Summary (function () { while(true) { } })() runs forever with DFG JIT
Csaba Osztrogonác
Reported 2012-02-20 11:30:56 PST
tst_QWebPage Qt API tests fails after enabling DFG JIT: QFATAL : tst_QWebPage::infiniteLoopJS() Received signal 15 FAIL! : tst_QWebPage::infiniteLoopJS() Received a fatal error. But unfortunately we didn't notice it before, because zillion API tests fail and nobody cares with them. And unfortunately timeouts were hidden before http://trac.webkit.org/changeset/108246 It might be a JSC bug, but we should check it on a small QWebPage independent example.
Attachments
Patch (1.94 KB, patch)
2012-02-20 22:51 PST, Csaba Osztrogonác 		no flags
DetailsFormatted DiffDiff
Proposed patch (7.20 KB, patch)
2012-09-27 09:36 PDT, Gabor Ballabas 		no flags
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Csaba Osztrogonác
Comment 1 2012-02-20 22:51:47 PST
Created attachment 127922 [details] Patch
Csaba Osztrogonác
Comment 2 2012-02-20 22:53:18 PST
(In reply to comment #1) > Created an attachment (id=127922) [details] > Patch It is a workaround to make our bots happier. Without fixing this bug or without this workaround our bots can't be green anymore. (because of stricter rule in master.cfg - http://trac.webkit.org/changeset/108246)
Simon Hausmann
Comment 3 2012-02-21 03:27:38 PST
Filip, it seems that LLint doesn't do the timeout checks for loops, like it's done in op_loop in Interpreter.cpp. Is that a missing feature in llint?
Simon Hausmann
Comment 4 2012-02-21 03:29:02 PST
Comment on attachment 127922 [details] Patch rs=me. Let's get this workaround in until we figure out why the timeout don't seem to be getting called with llint.
Csaba Osztrogonác
Comment 5 2012-02-21 04:24:58 PST
I made a little digging, and checked if TimeoutChecker::didTimeOut(ExecState* exec) is called or not. It is called from tst_QWebPage::infiniteLoopJS()if DFG JIT is disabled. But it isn't called if DFG JIT is enabled. And it isn't called with same infinite loop from DRT and QtTestBrowser irrespectively of DFG JIT is enabled or not.
Csaba Osztrogonác
Comment 6 2012-02-21 04:34:02 PST
Comment on attachment 127922 [details] Patch Clearing flags on attachment: 127922 Committed r108341: <http://trac.webkit.org/changeset/108341>
Zoltan Herczeg
Comment 7 2012-08-01 03:14:11 PDT
Hey Filip, I think I tracked down this issue. It seems the DFG JIT does not call TimeoutChecker at all, so nothing checks the timeout in case of a simple (function () { while(true) { } })() Which essentially stops the browser forever. Any idea how to fix this?
Gavin Barraclough
Comment 8 2012-09-21 01:12:10 PDT
We don't currently support a mechanism to asynchronously interrupt JavaScript execution. To stop a runaway script in WebKit2, you can kill the web process - this ensures all resources are released.
Zoltan Herczeg
Comment 9 2012-09-21 01:34:32 PDT
We discussed this issue with Philip before, and we decided that we refactor the code. This work is currently in progress.
Gabor Ballabas
Comment 10 2012-09-27 09:36:51 PDT
Created attachment 166022 [details] Proposed patch Proposed patch without changelog. It depends on https://bugs.webkit.org/show_bug.cgi?id=96916
Geoffrey Garen
Comment 11 2012-10-30 16:51:19 PDT
Mark Lam is working on a more efficient fix for this.
Zoltan Herczeg
Comment 12 2012-10-30 20:49:10 PDT
Thanks for letting us know!
Csaba Osztrogonác
Comment 13 2012-10-30 22:57:01 PDT
(In reply to comment #11) > Mark Lam is working on a more efficient fix for this. :(( I don't think if ignoring review at all for a month is the best / most friendlier way to let us know that you don't want Gábor's fix and you prefer fixing by yourself.
Geoffrey Garen
Comment 14 2012-10-31 08:52:34 PDT
> I don't think if ignoring review at all for a month is the best / > most friendlier way to let us know that you don't want Gábor's fix > and you prefer fixing by yourself. To clarify, Mark wasn't working on this a month ago. If you want this patch reviewed, the best thing to do is mark the review flag r? and post performance numbers for a reviewer to evaluate.
Gabor Ballabas
Comment 15 2012-11-13 07:20:38 PST
Sunspider benchmark results on x86_64 Linux: sunspider: TEST COMPARISON r134387 Proposed patch DETAILS ============================================================================= ** TOTAL **: *1.014x as slow* 208.5ms +/- 0.1% 211.4ms +/- 0.2% significant ============================================================================= 3d: *1.019x as slow* 32.6ms +/- 0.4% 33.2ms +/- 0.4% significant cube: *1.050x as slow* 11.5ms +/- 1.1% 12.1ms +/- 0.5% significant morph: ?? 9.1ms +/- 0.9% 9.3ms +/- 1.1% not conclusive: might be *1.012x as slow* raytrace: - 11.9ms +/- 0.5% 11.9ms +/- 0.5% access: ?? 18.0ms +/- 0.0% 18.0ms +/- 0.2% not conclusive: might be *1.001x as slow* binary-trees: - 2.0ms +/- 0.0% 2.0ms +/- 0.0% fannkuch: ?? 8.0ms +/- 0.0% 8.0ms +/- 0.3% not conclusive: might be *1.002x as slow* nbody: - 4.0ms +/- 0.0% 4.0ms +/- 0.0% nsieve: - 4.0ms +/- 0.0% 4.0ms +/- 0.0% bitops: 1.025x as fast 12.5ms +/- 0.8% 12.2ms +/- 0.7% significant 3bit-bits-in-byte: - 1.0ms +/- 0.0% 1.0ms +/- 0.0% bits-in-byte: 1.057x as fast 5.5ms +/- 1.8% 5.2ms +/- 1.6% significant bitwise-and: - 2.0ms +/- 0.0% 2.0ms +/- 0.0% nsieve-bits: - 4.0ms +/- 0.0% 4.0ms +/- 0.0% controlflow: - 2.0ms +/- 0.0% 2.0ms +/- 0.0% recursive: - 2.0ms +/- 0.0% 2.0ms +/- 0.0% crypto: - 17.0ms +/- 0.0% 17.0ms +/- 0.1% aes: - 10.0ms +/- 0.0% 10.0ms +/- 0.0% md5: ?? 4.0ms +/- 0.0% 4.0ms +/- 0.5% not conclusive: might be *1.002x as slow* sha1: - 3.0ms +/- 0.0% 3.0ms +/- 0.0% date: ?? 31.0ms +/- 0.1% 31.1ms +/- 0.5% not conclusive: might be *1.002x as slow* format-tofte: ?? 16.0ms +/- 0.2% 16.1ms +/- 0.8% not conclusive: might be *1.006x as slow* format-xparb: - 15.0ms +/- 0.2% 15.0ms +/- 0.3% math: *1.073x as slow* 18.6ms +/- 0.5% 20.0ms +/- 0.0% significant cordic: *1.33x as slow* 3.0ms +/- 0.0% 4.0ms +/- 0.0% significant partial-sums: - 13.0ms +/- 0.2% 13.0ms +/- 0.0% spectral-norm: *1.141x as slow* 2.6ms +/- 3.7% 3.0ms +/- 0.0% significant regexp: - 11.0ms +/- 0.2% 11.0ms +/- 0.0% dna: - 11.0ms +/- 0.2% 11.0ms +/- 0.0% string: *1.017x as slow* 65.8ms +/- 0.2% 66.9ms +/- 0.4% significant base64: ?? 5.0ms +/- 0.0% 5.0ms +/- 0.4% not conclusive: might be *1.002x as slow* fasta: - 10.0ms +/- 0.0% 10.0ms +/- 0.0% tagcloud: *1.008x as slow* 16.2ms +/- 0.5% 16.4ms +/- 0.6% significant unpack-code: *1.025x as slow* 26.5ms +/- 0.4% 27.1ms +/- 0.6% significant validate-input: *1.042x as slow* 8.1ms +/- 0.6% 8.4ms +/- 1.5% significant V8: TEST COMPARISON r134387 Proposed patch DETAILS ============================================================================= ** TOTAL **: *1.016x as slow* 748.9ms +/- 0.0% 760.7ms +/- 0.1% significant ============================================================================= v8: *1.016x as slow* 748.9ms +/- 0.0% 760.7ms +/- 0.1% significant crypto: *1.107x as slow* 86.8ms +/- 0.1% 96.1ms +/- 0.1% significant deltablue: *1.009x as slow* 135.4ms +/- 0.1% 136.7ms +/- 0.1% significant earley-boyer: *1.006x as slow* 97.8ms +/- 0.1% 98.4ms +/- 0.2% significant raytrace: *1.002x as slow* 70.8ms +/- 0.1% 71.0ms +/- 0.1% significant regexp: 1.010x as fast 107.2ms +/- 0.1% 106.1ms +/- 0.1% significant richards: *1.014x as slow* 113.5ms +/- 0.2% 115.0ms +/- 0.2% significant splay: - 137.3ms +/- 0.2% 137.4ms +/- 0.3%
Csaba Osztrogonác
Comment 16 2012-11-21 06:30:00 PST
As far as I know, Gábor doesn't work on it anymore. Could you confirm it?
Gabor Ballabas
Comment 17 2012-11-21 06:33:36 PST
(In reply to comment #16) > As far as I know, Gábor doesn't work on it anymore. Could you confirm it? Yes, that's true. As far as I know Mark Lam is working on it now.
Note You need to log in before you can comment on or make changes to this bug.