Safari often freezes when clicking "Return free memory" in Caches dialog

• Component: JavaScriptCore
• Status: RESOLVED FIXED

[Michael Saboff]

From <rdar://problem/8009651>.

5/20/10 11:46 AM Alexey Proskuryakov:
I'm seeing this after running DOM Hanoi test Steps to reproduce:

1. Run DOM Hanoi test <https://safari.apple.com/groups/safariteam/wiki/bb053/attachments/febd2/DOM-Hanoi%20v0.2.html>. Be sure to set recursion limit to 2, because 3 is way too slow.
2. After it's done, click "Return free memory" button in Caches window.
3. If that doesn't work, try forcing GC, closing the DOM Hanoi window, and going back to step 2.

Spin log attached.

6/17/10 4:42 PM Mark Rowe:
In a debug build using FastMalloc I hit this assertion:

0x0000000101bbf541 in WTF::TCMalloc_PageHeap::Carve (this=0x101eccdc0, span=0x113af1470, n=1, released=true) at FastMalloc.cpp:1701
1701	    ASSERT(span->decommitted);

I think the tracking of committed vs decommitted spans is incorrect, leading to ::scavenge being unable to find all of the free committed spans that it intends to release, which causes it to loop indefinitely with the page heap lock held.

5/23/11 10:29 AM Michael Saboff:
I think there are two problems here.  The assertion failure appears to be as a result of http://trac.webkit.org/changeset/58346.  The freeze seems to be a result of http://trac.webkit.org/changeset/58730.  Both of these changes were added as part of <rdar://problem/7834433>.

Investigating the assertion failure, but I believe the hang is due to the loop in TCMalloc_PageHeap::scavenge() becoming an infinite loop due to not being able to release enough free pages to get below "targetPageCount".

5/23/11 4:39 PM Michael Saboff:
The assertion failure is due to missing the setting of the "decommitted" flag in "s" in ReleaseFreeList() before the call to DLL_Prepend(returned, s).  Even with that fix, the hang can still happen.  Therefore I coded a fix for the hang as well.

[Michael Saboff]

Created attachment 94523 [details]
Patch with 2 fixes and sanity check improvement

[WebKit Review Bot]

Attachment 94523 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/ChangeLog', u'Source..." exit_code: 1

Source/JavaScriptCore/wtf/FastMalloc.cpp:1360:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/JavaScriptCore/wtf/FastMalloc.cpp:1360:  min_pages is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/wtf/FastMalloc.cpp:1360:  max_pages is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/wtf/FastMalloc.cpp:2122:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/JavaScriptCore/wtf/FastMalloc.cpp:2123:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/JavaScriptCore/wtf/FastMalloc.cpp:2132:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
Source/JavaScriptCore/wtf/FastMalloc.cpp:2132:  TCMalloc_PageHeap::CheckList is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/wtf/FastMalloc.cpp:2136:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
Source/JavaScriptCore/wtf/FastMalloc.cpp:2136:  TCMalloc_PageHeap::CheckList is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/wtf/FastMalloc.cpp:2136:  min_pages is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/wtf/FastMalloc.cpp:2136:  max_pages is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Total errors found: 11 in 2 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Michael Saboff]

Committed r87157: <http://trac.webkit.org/changeset/87157>