Last Comment Bug 444260 - Wide <xul:button> causes nsNativeThemeCocoa::DrawCellWithScaling to autorelease a freed object
: Wide <xul:button> causes nsNativeThemeCocoa::DrawCellWithScaling to autorelea...
Status: VERIFIED FIXED
: [sg:critical?]
: crash, testcase, verified1.9.0.4, verified1.9.1
Product: Core
Classification: Components
Component: Widget: Cocoa
: Trunk
: x86 Mac OS X
: P1 critical (vote)
: ---
Assigned To: Steven Michaud
: cocoa
:
:
: 344486 435223
  Show dependency treegraph
 
Reported: 2008-07-08 20:34 PDT by Jesse Ruderman
Modified: 2009-01-14 12:37 PST (History)
8 users (show)
joshmoz: blocking1.9.1+
dveditz: blocking1.9.0.4+
samuel.sidler+old: wanted1.9.0.x+
dveditz: wanted1.8.1.x-
jruderman: in‑testsuite+
See Also:
Crash Signature:


Attachments
testcase (crashes Firefox when loaded) (143 bytes, application/vnd.mozilla.xul+xml)
2008-07-08 20:34 PDT, Jesse Ruderman
no flags Details
Gdb trace of crash (with debug symbols) (8.69 KB, text/plain)
2008-07-09 07:35 PDT, Steven Michaud
no flags Details
Trivial fix (somewhat ugly) (3.19 KB, patch)
2008-07-09 09:14 PDT, Steven Michaud
no flags Details | Diff | Splinter Review
Less ugly fix (still pretty trivial) (3.57 KB, patch)
2008-07-09 09:34 PDT, Steven Michaud
no flags Details | Diff | Splinter Review

Summon comment box

Description Jesse Ruderman 2008-07-08 20:34:04 PDT
Created attachment 328616 [details]
testcase (crashes Firefox when loaded)

Version: trunk debug build on Leopard

Loading the testcase causes a malloc error "can't allocate region" (perhaps bug 435223), and then:

objc[20992]: FREED(id): message autorelease sent to freed object=0x1ca0afe0

The malloc error doesn't bother me too much, but touching freed objects is pretty bad.  In fact, with MallocScribble enabled, Firefox dereferences 0x55555575 instead of triggering the error message above.
Comment 1 Steven Michaud 2008-07-09 07:35:23 PDT
Created attachment 328675 [details]
Gdb trace of crash (with debug symbols)

Here's a gdb trace of this crash, made with an opt 1.9.0-branch build
containing debug symbols.  It has both console messages and a stack
trace.

I'll be working on this.
Comment 2 Steven Michaud 2008-07-09 09:14:15 PDT
Created attachment 328691 [details] [review]
Trivial fix (somewhat ugly)

Here's a trivial patch that fixes the crash -- it refuses to draw when
CGBitmapContextCreate() fails (when it returns NULL).

But the impossibly large button does get drawn on other platforms
(e.g. Windows and Linux), so I suppose we should try to work some kind
of fallback into the drawing code, so that it will still "work" even
when it's called with parameters that are far too large.

Since I'm not very familiar with this code, I'd prefer to leave this
to others.

Josh? :-)
Comment 3 Steven Michaud 2008-07-09 09:34:03 PDT
Created attachment 328698 [details] [review]
Less ugly fix (still pretty trivial)

On second thought, maybe I _can_ do better.

Here's a slightly less trivial patch that's considerably less ugly.
You still see errors in the console (which I think is entirely
appropriate), but the button does get drawn (and in the same way as on
Windows and Linux).
Comment 4 Steven Michaud 2008-07-14 16:48:35 PDT
Comment on attachment 328698 [details] [review]
Less ugly fix (still pretty trivial)

This patch is (I think) superceded by my current patch for bug 444864
(attachment 329570 [details] [review]).
Comment 5 Steven Michaud 2008-08-21 13:32:53 PDT
The autorelease-after-deletion problem is an Apple bug.  See bug 449111
comment #5.
Comment 6 Daniel Veditz 2008-09-10 15:19:28 PDT
Does that mean we can't fix it? Can we work around it?
Is there an Apple bug reference for the problem?
Comment 7 Steven Michaud 2008-09-10 16:07:26 PDT
We can work around the problem.  The patch for bug 444864 does so.
Comment 8 Steven Michaud 2008-09-15 07:51:15 PDT
Fixed by patch for bug 444864, which was just landed on mozilla-central.
Comment 9 Steven Michaud 2008-09-15 11:25:49 PDT
Reopened because I've backed out my patch for bug 444864 (which
probably caused some reftest failures).
Comment 10 Steven Michaud 2008-09-17 08:23:42 PDT
Fixed by my new patch for bug 444864, which was just landed on mozilla-central.
Comment 11 Jesse Ruderman 2008-09-17 14:22:21 PDT
Crashtest is in (bug 444864 comment 26).
Comment 12 Daniel Veditz 2008-09-17 15:28:51 PDT
I assume this isn't needed for Firefox 2 because it's cocoa? I'm that's incorrect please nominate for blocking 1.8.1.x
Comment 13 Steven Michaud 2008-09-17 16:32:00 PDT
This bug (bug 444260), bug 444864 and bug 449111 only effect (happen on)
Firefox 3.X -- not Firefox 2.
Comment 14 Steven Michaud 2008-10-13 09:32:30 PDT
Fixed on the 1.9.0 branch by the patch for bug 444864.
Comment 15 Marcia Knous [:marcia] 2008-10-13 16:24:56 PDT
verified fixed using Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1b2pre) Gecko/20081013 Minefield/3.1b2pre. I verified by using the testcase in Comment 0.
Comment 16 Al Billings [:abillings] 2008-10-21 15:37:20 PDT
Verified for 1.9.0.4 with Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.0.4pre) Gecko/2008102104 GranParadiso/3.0.4pre.

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