331077 - nsFontMetricsXft::CacheFontMetrics() : face may be NULL [@ nsFontMetricsXft::CacheFontMetrics]

Status: VERIFIED FIXED

(Core Graveyard :: GFX: Gtk, defect)

Description

[Tobias Ulmer]

User-Agent:       Mozilla/5.0 (X11; U; OpenBSD i386; en-US; rv:1.8.0.1) Gecko/20060318 Firefox/1.5.0.1
Build Identifier: Mozilla/5.0 (X11; U; OpenBSD i386; en-US; rv:1.8.0.1) Gecko/20060318 Firefox/1.5.0.1

Under some circumstances, XftLockFace() can return NULL (See XftLockFace sourcecode...). This results in a nullpointer dereference and obviously in a crash.

There was a bug report about this here: https://bugzilla.mozilla.org/show_bug.cgi?id=183729, but it was closed as a duplicate of this bug: https://bugzilla.mozilla.org/show_bug.cgi?id=180309. Since this bug description is not really appropriate (It's not cause by unreadable fonts), I open a new bugreport.

Two patches are available:
http://arkiv.netbsd.se/?ml=openbsd-ports&a=2006-03&m=1852803 or https://bugzilla.mozilla.org/attachment.cgi?id=140288

This bug also applies to the Mozilla Suite.

Reproducible: Sometimes

Steps to Reproduce:
Browse on zdnet or ebay... (May have something to do with the minimum fontsize and changed default fonts, however my setup is to complex to reproduce this and it's a complete waste of time. XftLockFace() is specified to return NULL and the code should act uppon it, so just fix it)
Actual Results:  
Crash. (Segfault)

Expected Results:  
Not to crash ;)

--with-system-jpeg=/usr/local --with-system-png=/usr/local --with-system-zlib=/usr/lib --with-pthreads --without-system-nspr --enable-xft --enable-optimize=-Os --enable-default-toolkit=gtk2 --disable-debug --disable-tests --disable-pedantic --disable-installer --disable-updater --disable-gnomeui --disable-gnomevfs --enable-xinerama --enable-svg --enable-svg-renderer=cairo --enable-system-cairo --enable-canvas --enable-application=browser --prefix=/usr/local --sysconfdir=/etc

Comment 1

[David Baron :dbaron: (⌚️UTC-5, no longer working on Mozilla)]

There's a patch that will allegedly fix this on bug 183729.

Comment 2

[David Baron :dbaron: (⌚️UTC-5, no longer working on Mozilla)]

*** This bug has been marked as a duplicate of 183729 ***

Comment 3

[David Baron :dbaron: (⌚️UTC-5, no longer working on Mozilla)]

Marking this duplicate was a bad idea, that other bug was originally something else, but just happened to have a patch that would fix this one.

This is a topcrash for Linux Firefox 1.5.0.2.

There's a patch on the bug 183729 for it.

Comment 4

[David Baron :dbaron: (⌚️UTC-5, no longer working on Mozilla)]

Patch there checked in to trunk and MOZILLA_1_8_BRANCH.

I think we need to make sure that failure returns from this function don't crash elsewhere, though.  I'm not confident that that's the case...

Comment 5

[David Baron :dbaron: (⌚️UTC-5, no longer working on Mozilla)]

Yeah, I'm thinking that what we really need to do is make checking that XftLockFace succeeds be part of GetXftFont so that FindFont doesn't return a font for which it fails.

Comment 6

[David Baron :dbaron: (⌚️UTC-5, no longer working on Mozilla)]

Thing is, we call XftLockFace only on mWesternFont, not on all of them, so I wouldn't want to waste the time later.  (Or if XftLockFace fails, would other uses of the font also fail?)

Comment 7

[David Baron :dbaron: (⌚️UTC-5, no longer working on Mozilla)]

What I don't understand is how XftLockFace could return null when XftFontOpenInfo has already returned non-null.

That said, we should probably call XftUnlockFace at some point...

Comment 8

[David Baron :dbaron: (⌚️UTC-5, no longer working on Mozilla)]

(In reply to comment #7)
> That said, we should probably call XftUnlockFace at some point...

Er, never mind, we already do.

Comment 9

[David Baron :dbaron: (⌚️UTC-5, no longer working on Mozilla)]

Setting back to fixed -- based on looking at the Xft code, this is a situation that basically shouldn't happen unless we run out of memory, although that may not have been true for older versions of Xft.  In any case, we *do* handle the error, as long as it's not the first font in the font cache, thanks to what nsFontCache::GetMetricsFor does when nsIFontMetrics::Init fails.

But I suspect something was different for older versions of Xft, or I'm misreading something, since users are actually hitting it...

Comment 10

[Tracy Walker [:tracy]]

no longer seeing on topcrash reports