Bug 28304 – S60 Qt Webkit browser crashes while loading the V8 Benchmark Suite

• Component: JavaScriptCore
• Status: RESOLVED WONTFIX

[Chang Shu]

Steps to reproduce:
1.Load the page http://v8.googlecode.com/svn/data/benchmarks/v4/run.html
2.The automated tests start running.

Expected result:
1.All the tests run and the score is displayed

Actual result:
Browser crashes around 87% complete

Root cause:
JavascriptCore heap memory exhausted

[Chang Shu]

Created attachment 34839 [details]
This is the patch to fix the bug.

[Eric Seidel (no email)]

I don't understand this hack.  Why can't we fix this for real?

[Oliver Hunt]

Comment on attachment 34839 [details]
This is the patch to fix the bug.

The Interpreter should not have any platform specific code in it.

In general if your patch adds #if PLATFORM(...) to code where there are no existing PLATFORM specific #ifs, your patch is wrong.

I'm also unsure why you feel the need to do this here -- there are plenty of places where you can just check for malloc failure and convert to a JS OOM exception.

[Simon Hausmann]

Chang, any update on this? (this is currently marked as critical bug...)

[Chang Shu]

(In reply to comment #4)
> Chang, any update on this? (this is currently marked as critical bug...)

I put it on hold for a long time after my 1st patch was turned down. I started to work on it again recently as it is critical to find a new solution for the crash. I am working on a solution based on Oliver's suggestion: Do oom check and throw oom exceptions at the places closer to the mallocs. The disadvantage is it requires many more code changes but this should be the correct way of fixing the problem. I probably need another several days before I submit the next patch.

[Oliver Hunt]

The problems with this patch are:
  * It adds platform specific ifdefs to non-platform specific code;  If we want to not crash on OOM we would want it fixed for all platforms
  * The method for "detecting" low memory is flawed in that it is basically periodically sampling the GC heap to see how big it is.  Given that a significant amount of JSC (and WebKit) usage from GC objects is allocated independently from the GC this sampling doesn't actually fix the problem, it merely papers over it.
  * The fix implies a compiler that does not support computed goto or the jit both of which will never see this check.

Just to clarify it is intended behaviour that javascriptcore and webkit crash in low/no memory circumstances, this is a security feature -- we would rather crash immediately than potentially allow null values into the engine.

[Chang Shu]

(In reply to comment #6)
> The problems with this patch are:
>   * It adds platform specific ifdefs to non-platform specific code;  If we want
> to not crash on OOM we would want it fixed for all platforms
>   * The method for "detecting" low memory is flawed in that it is basically
> periodically sampling the GC heap to see how big it is.  Given that a
> significant amount of JSC (and WebKit) usage from GC objects is allocated
> independently from the GC this sampling doesn't actually fix the problem, it
> merely papers over it.
>   * The fix implies a compiler that does not support computed goto or the jit
> both of which will never see this check.
> 
> Just to clarify it is intended behaviour that javascriptcore and webkit crash
> in low/no memory circumstances, this is a security feature -- we would rather
> crash immediately than potentially allow null values into the engine.

Thanks for the update, Oliver. I will see if we (S60 QtWebKit people) can drop this code change as you said the crash is intentional. Actually, we also think it is acceptable to let application crash in low/no memory condition. We want this V8 crash to be fixed as it blocks the performance testing.

[Chang Shu]

Based on the feedback from the reviewers and discussion with people in our organization, we decide the current behavior(crash) is desired in OOM situation. Thus, no code change is expected.

[Siddharth Mathur]

Since khansen asked, Bug 34350 fixed this issue in Qt 4.6.x's QtWebkit (no crash at all) as long as (a) a custom (more efficient than default) memory allocator was used for the process (b) large enough max heap size was specified, (c) device runs Symbian^3