Last Comment Bug 439206 - [FIX]Shutdown crash [@ PL_DHashTableFinish] with high surrogate in <style>
: [FIX]Shutdown crash [@ PL_DHashTableFinish] with high surrogate in <style>
Status: RESOLVED FIXED
: [sg:critical?]
: crash, fixed1.9.1, regression, testcase, verified1.9.0.4
Product: Core
Classification: Components
Component: XPCOM
: Trunk
: x86 Mac OS X
: P1 normal (vote)
: ---
Assigned To: Boris Zbarsky (:bz)
: xpcom
:
:
: 377438 490513
  Show dependency treegraph
 
Reported: 2008-06-14 01:37 PDT by Jesse Ruderman
Modified: 2009-06-08 13:13 PDT (History)
9 users (show)
benjamin: blocking1.9.1+
dveditz: blocking1.9.0.4+
jruderman: in‑testsuite+
See Also:
Crash Signature:
[@ PL_DHashTableFinish]


Attachments
testcase (makes Firefox crash on shutdown) (331 bytes, text/html)
2008-06-14 01:37 PDT, Jesse Ruderman
no flags Details
Fix (1.09 KB, patch)
2008-09-02 14:14 PDT, Boris Zbarsky (:bz)
jst: review+
jst: superreview+
dveditz: approval1.9.0.4+
Details | Diff | Splinter Review

Summon comment box

Description Jesse Ruderman 2008-06-14 01:37:48 PDT
Created attachment 325076 [details]
testcase (makes Firefox crash on shutdown)

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.4; en-US; rv:1.9.1a1pre) Gecko/2008061310 Minefield/3.1a1pre

Loading the testcase and then quitting Firefox (Cmd+Q) usually results in a crash:

* PL_DHashTableFinish calling a random address
* PL_DHashTableFinish calling AtomTableClearEntry, which dereferences a random address

The problem clearly starts with bug 316338, but I think there's a recent regression in how the style system or the atom table deals with it.
Comment 1 Jesse Ruderman 2008-06-25 10:39:45 PDT
I'm hitting this crash frequently enough that it interferes with fuzzing.
Comment 2 Jesse Ruderman 2008-07-07 13:03:17 PDT
jst, can you look at this?
Comment 3 Boris Zbarsky (:bz) 2008-09-02 14:14:36 PDT
Created attachment 336544 [details] [review]
Fix

The issue was that we added the atom to the table with one hashcode (as computed via HashCodeAsUTF8) but tried to remove it using the hashcode of the UTF-8 string stored in the atom when the atom went away.  These should be the same, but there was a bug in HashCodeAsUTF8 that caused them to differ in the missing-low-surrogate case, which left a pointer to the dead atom in the atom table, and hence a shutdown crash.

This patch just fixes that bug, making this code consistent with what the ConvertUTF16toUTF8 function and the UTF16CharEnumerator do.

We probably want this patch on 1.9.0.x.
Comment 4 Boris Zbarsky (:bz) 2008-09-04 09:48:30 PDT
Pushed changeset a06a5b54d548.
Comment 5 Daniel Veditz 2008-09-10 15:34:36 PDT
Comment on attachment 336544 [details] [review]
Fix

Approved for 1.9.0.3, a=dveditz for release-drivers
Comment 6 Boris Zbarsky (:bz) 2008-09-12 06:38:20 PDT
Fixed on branch.
Comment 7 Al Billings [:abillings] 2008-10-21 15:28:30 PDT
I cannot get 3.0.3 to crash with this test case on either OS X or Windows XP. How reliable is the crash?
Comment 8 Boris Zbarsky (:bz) 2008-10-21 17:26:07 PDT
It was about every other time or so for me on trunk with a debug build...  I suspect opt builds it would happen less commonly.
Comment 9 Aakash Desai [:aakashd] 2009-04-14 10:09:26 PDT
Seeing as there hasn't been any discussions about this bug for 5 1/2 months and it's been in mochitest for that long, I'm assuming there aren't any residual issues. I'm moving this to verified as a result. If anyone has any qualms, feel free to bring them up.
Comment 10 Samuel Sidler (old account; do not CC) 2009-04-14 10:43:14 PDT
You can verify this with a debug build, per comment 8.
Comment 11 Al Billings [:abillings] 2009-04-14 11:31:44 PDT
I verified this for 1.9.0 using Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.0.10pre) Gecko/2009040612 Minefield/3.0.10pre (my own debug build from last week).

Note You need to log in before you can comment on or make changes to this bug.