Skip to content
This repository has been archived by the owner on May 30, 2023. It is now read-only.

PhantomJS crashes on OS X when handling certain TTF fonts #10690

Closed
aaronjensen opened this issue Aug 2, 2012 · 31 comments
Closed

PhantomJS crashes on OS X when handling certain TTF fonts #10690

aaronjensen opened this issue Aug 2, 2012 · 31 comments
Milestone

Comments

@aaronjensen
Copy link

aaronjen...@gmail.com commented:

Which version of PhantomJS are you using? Tip: run 'phantomjs --version'.

1.6.1

Did you use binary PhantomJS or did you compile it from source?

Binary, installed w/ brew install phantomjs

Please provide any additional information below.

I was running tests with capybara/poltergeist

Disclaimer:
This issue was migrated on 2013-03-15 from the project's former issue tracker on Google Code, Issue #690.
🌟   19 people had starred this issue at the time of migration.

@ariya
Copy link
Owner

ariya commented Aug 4, 2012

joniscoo...@googlemail.com commented:

Crash dump: https://gist.github.com/3257878

@ariya
Copy link
Owner

ariya commented Aug 4, 2012

joniscoo...@googlemail.com commented:

Looks like the same issue as teampoltergeist/poltergeist#44 (something to do with TTF fonts)

@ariya
Copy link
Owner

ariya commented Aug 9, 2012

joniscoo...@googlemail.com commented:

 

 
Metadata Updates

  • Title updated: PhantomJS crashes on OS X when handling certain TTF fonts

@ariya
Copy link
Owner

ariya commented Aug 9, 2012

joniscoo...@googlemail.com commented:

Issue 677 has been merged into this issue.

@mhenrixon
Copy link

mik...@zoolutions.se commented:

Unfortunately the truetype font issue makes phantomjs completely unusable on Mac OS X Mountain Lion.

@namxam
Copy link

namxam commented Sep 25, 2012

nam...@gmail.com commented:

Still crashes with 1.7.0

@ariya
Copy link
Owner

ariya commented Sep 27, 2012

ariya.hi...@gmail.com commented:

Is this with fontawesome-webfont.ttf? Someone should create a simple unit test so that it can get included in our test suite.

@ariya
Copy link
Owner

ariya commented Nov 3, 2012

team@bitium.com commented:

The problem occurs with any .ttf font -- see my last comment on teampoltergeist/poltergeist#44

I created a test repo @ https://github.com/goosetav/true_type_bug using free TTF font from Font Squirrel and can reproduce the bug by driving PJS with poltergeist.

I'm happy to run some tests locally to try and reproduce.

@ariya
Copy link
Owner

ariya commented Dec 26, 2012

ariya.hi...@gmail.com commented:

I have created a simple test page: http://ariya.github.com/html/webfont/. It loads both Tangerine WOFF and Font Awesome (via its official CSS). When running our rasterize example, PhantomJS 1.8 does not crash on OS X.

I changed the Tangerine WOFF with Windsong TTF (from https://github.com/goosetav/true_type_bug/blob/master/app/assets/fonts/Windsong-webfont.ttf) and I still can't crash it.

 
Metadata Updates

  • Label(s) added:
    • Component-Logic
    • Domain-Qt
  • Milestone updated: FutureRelease (was: ---)

@ariya
Copy link
Owner

ariya commented Dec 26, 2012

ariya.hi...@gmail.com commented:

Just another update. I can reproduce the crash from https://github.com/goosetav/true_type_bug when running 'bundle exec rspec. I'm now trying to find a way to crash PhantomJS directly for an easier debugging. Any hint will be appreciated!

 
Metadata Updates

  • Status updated: Accepted

@ariya
Copy link
Owner

ariya commented Dec 26, 2012

ariya.hi...@gmail.com commented:

An exemplary crash dump (PhantomJS 1.8.0, Lion) on https://github.com/goosetav/true_type_bug test case.

@ariya
Copy link
Owner

ariya commented Dec 26, 2012

ariya.hi...@gmail.com commented:

The decoded stack trace is attached (I only included the thread which crashed).

The only thing related to font in that trace is:

phantomjs!__ZN7WebCore10CachedFont17ensureSVGFontDataEv + 0x2ac

@goosetav
Copy link

e...@bitium.com commented:

i just committed a couple scripts that will reproduce the crash directly from PhantomJS to https://github.com/goosetav/true_type_bug

running "phantomjs crash-phantom.js" reproduces the bug

@ariya
Copy link
Owner

ariya commented Dec 27, 2012

ariya.hi...@gmail.com commented:

Thanks a lot, Erik! The simple test is really helpful.

One thing I notice that it basically boils down the font being used. A simpler HTML content like:

Hi

would also trigger the crash.

 
Metadata Updates

  • Label(s) removed:
    • Priority-Medium
  • Label(s) added:
    • Priority-High

@ariya
Copy link
Owner

ariya commented Dec 27, 2012

ariya.hi...@gmail.com commented:

Partial stack trace of the crashing thread:

#0 0x9254bd47 in objc_msgSend ()
#1 0x0267b7b0 in ?? ()
#2 0x9a9b8310 in TCFRetained<CGFont*>::Retain ()
#3 0x9a947102 in TCGFont::TCGFont ()
#4 0x9a946dee in TCGFontCache::CopyFont ()
#5 0x9a946ca8 in TBaseFont::CopyNativeFont ()
#6 0x9a955338 in CTFontCopyGraphicsFont ()
#7 0x00bace39 in QCoreTextFontEngine::QCoreTextFontEngine ()
#8 0x00bae0cb in QCoreTextFontEngineMulti::init ()
#9 0x00baa530 in QCoreTextFontEngineMulti::QCoreTextFontEngineMulti ()
#10 0x00ddb247 in QFontDatabase::load ()
#11 0x00dc3e55 in QFontPrivate::engineForScript ()
#12 0x00dd22da in QFontMetricsF::descent ()
#13 0x00692f1b in WebCore::SimpleFontData::platformInit ()
#14 0x00553cad in WebCore::SimpleFontData::SimpleFontData ()
#15 0x0023cf2e in WebCore::CSSFontFaceSource::getFontData ()
#16 0x0023702c in WebCore::CSSFontFace::getFontData ()

Apparently, when QCoreTextFontEngine wanted to process Windsong font for the second time (i.e. another page load), CTFontCopyGraphicsFont did not seem very happy and then it crashed. I'm not sure exactly why.

@goosetav
Copy link

e...@bitium.com commented:

I also noticed the release() method seems instigate the crash. I added a second script to my repo that uses two vars instead of redefining the same var. It will still crash if .release() is called on the first page object.

Not knowing anything about the internals of PhantomJS, perhaps the font has a reference to the original page (or its child objects) that becomes invalid after a release() is called?

https://github.com/goosetav/true_type_bug/blob/master/does-not-crash-phantom.js

@goosetav
Copy link

e...@bitium.com commented:

I added another script that does not cause the crash - it doesn't call release() on the first page object but do call release() on the 2nd page object created.

When visiting the first page, is CTFontCopyGraphicsFont invoked as well or is that only occurring when loading a font from a cache?

I wonder if forcing the font files to not cache would also bypass the crash?

@ariya
Copy link
Owner

ariya commented Dec 28, 2012

ariya.hi...@gmail.com commented:

deleteLater() inside WebPage::close() is the trigger for the crash. It seems that there is something wrong with the actual page clean-up. However, coming to think about it again, our way of deleting the web page object may be responsible for other types of crashes I've seen in this issue tracker.

@ariya
Copy link
Owner

ariya commented Dec 28, 2012

ariya.hi...@gmail.com commented:

I concur that calling release() before opening the same URL triggers the crashing behavior. Something like:

var page1 = require('webpage').create();
var page2 = require('webpage').create();

page1.open(url, function () {
page1.release();
page2.open(url, function () {
phantom.exit();
});
});

@goosetav
Copy link

e...@bitium.com commented:

but interestingly, this does not cause a crash -- only release() on the first webpage object for a given URL triggers it

var page1 = require('webpage').create();
var page2 = require('webpage').create();
var page3 = require('webpage').create();

page1.open(url, function () {
//page1.release(); //releasing here will trigger a crash
page2.open(url, function () {
page2.release();
page3.open(url, function () {
phantom.exit();
});
});
});

@ariya
Copy link
Owner

ariya commented Dec 28, 2012

ariya.hi...@gmail.com commented:

Disabling memory cache or even enabling the use of "private browser" did not really help. Somehow this relates to Qt font engine on OS X (hence why the issue does not show up in other platforms) and only for some particular fonts.

@ariya
Copy link
Owner

ariya commented Dec 29, 2012

ariya.hi...@gmail.com commented:

Possible upstream bug: https://bugs.webkit.org/show_bug.cgi?id=61031.

@ariya
Copy link
Owner

ariya commented Dec 30, 2012

ariya.hi...@gmail.com commented:

A little bit of what happens under the hood.

Everytime a font needs to be handled (e.g. looking up its metrics, drawing a text using that font, etc), a query is made to the Qt font database. This database has a cache mechanism to minimize creating the font data and platform-specific font engine for every possible font combination of family, weight, style, size, etc. The font cache is however invalidated (and thus becomes completely empty) every time a new application-specific font is added or removed via QFontDatabase::addApplicationFont and QFontDatabase::removeApplicationFont, respectively.

A font engine is always platform specific. On Mac, Qt leverages Core Text and Core Graphics and hence why the calls to CTFontCopyGraphicsFont in the stack trace. The sequence that leads to the crash is:

WebCore::createFontCustomPlatformData
  QFontDatabase::addApplicationFontFromData (Windsong TTF loaded by WebKit)
    QFontDatabasePrivate::invalidate
      QFontCache::clear
        Destroy QCoreTextFontEngine Windsong
QFontDatabase::load for Windsong 90 px
  QCoreTextFontEngine   Create font for Windsong 0x25622f0
  QFontDatabase Inserting font engine in the cache

---- page.release() is invoked and we load the URL the second time ---

WebCore::FontCustomPlatformData::~FontCustomPlatformData  // part of old page destruction
  QFontDatabase::removeApplicationFontQFontDatabase::removeApplicationFont 0
    QFontDatabasePrivate::invalidate
      QFontCache::clear
        Destroy QCoreTextFontEngine Windsong

WebCore::createFontCustomPlatformData // Windsong from the new page
  QFontDatabase::addApplicationFontFromData (Windsong TTF loaded by WebKit)
    QFontDatabasePrivate::invalidate
      QFontCache::clear
QFontDatabase::load for Windsong 90 px
  QCoreTextFontEngine   Create font for Windsong 0x25622f0  // CRASH here

What I am playing with right now is the workaround to "break" this crash sequence, i.e. preventing it from happening at the first place. It is possible that we may pay a small price because of the workaround.

@ariya
Copy link
Owner

ariya commented Dec 30, 2012

ariya.hi...@gmail.com commented:

After some careful thinking, my best workaround is as follows:

--- a/src/qt/src/3rdparty/webkit/Source/WebCore/platform/graphics/qt/FontCustomPlatformDataQt.cpp
+++ b/src/qt/src/3rdparty/webkit/Source/WebCore/platform/graphics/qt/FontCustomPlatformDataQt.cpp
@@ -32,7 +32,11 @@ namespace WebCore {

FontCustomPlatformData::~FontCustomPlatformData()
{
+#if !defined(Q_OS_MAC)

The drawbacks I can think of:

(1) We intentionally leak the font engine for custom fonts specified in the web page (but not for system-installed fonts). This is not so bad as we don't often encounter web sites with hundreds of custom font. The max I found so far is theverge.com with 6 fonts. ESPN, NYTimes, BBC, and other popular new sites won't leak anything.

(2) Two custom fonts may cause conflict. For example, if a web page defines "FooBar" font and then another web page has the same font name but with totally different typography (i.e. the text look different). This is not a new problem, since Qt defines custom fonts at the application level, it already suffers from that behavior (though in this case only when two pages are opened at the same time).

I believe this is a compromise we can live with, at least for the time being until someone acquires a better understanding of why this happens and how to prevent it the "right way".

@ariya
Copy link
Owner

ariya commented Dec 30, 2012

ariya.hi...@gmail.com commented:

My post to the mailing-list, to summarize the problem: https://groups.google.com/d/topic/phantomjs/FKqXBagjNjU/discussion.

@ariya
Copy link
Owner

ariya commented Dec 31, 2012

ariya.hi...@gmail.com commented:

Mac OS X: Add a manual test for the crash with TrueType fonts.
2a2b6e455c

Mac OS X: Fix possible crash when using some TrueType fonts.
d0fe6864a9

 
Metadata Updates

  • Milestone updated: Release1.9 (was: FutureRelease)
  • Status updated: Fixed

@ariya ariya closed this as completed Dec 31, 2012
@ariya
Copy link
Owner

ariya commented Dec 31, 2012

ariya.hi...@gmail.com commented:

As I mentioned in my mailing-list post, I will backport this to 1.8 branch for the upcoming 1.8.1 release.

Erik, thanks again for the excellent test case!

@ariya
Copy link
Owner

ariya commented Jan 5, 2013

ariya.hi...@gmail.com commented:

Issue 959 has been merged into this issue.

@ariya
Copy link
Owner

ariya commented Jan 12, 2013

ariya.hi...@gmail.com commented:

Issue 961 has been merged into this issue.

@ariya
Copy link
Owner

ariya commented Jan 12, 2013

ariya.hi...@gmail.com commented:

Issue 978 has been merged into this issue.

@ariya
Copy link
Owner

ariya commented Jan 13, 2013

ariya.hi...@gmail.com commented:

Issue 945 has been merged into this issue.

zackw added a commit to zackw/phantomjs that referenced this issue Aug 7, 2015
NOTE: This test used to include a custom webserver process that
sent a bunch of response headers which were supposed to prevent
all caching.  Examination of issue ariya#10690 indicates this was
*probably* not necessary to trigger the bug, so I have dropped
it rather than add equivalent functionality to the webserver in
run-tests.py.

(This test was not being run automatically.)
zackw added a commit to zackw/phantomjs that referenced this issue Aug 7, 2015
Also: Better naming convention for tests in test/regression/.

regression/pjs-12482.js should not use the harness, because it tests
phantom.exit.

set/690-ttf-crash/run.js is a regression test hiding off to the side;
include it.  NOTE: This test used to include a custom webserver process
that sent a bunch of response headers which were supposed to prevent
all caching.  Examination of issue ariya#10690 indicates this was *probably*
not necessary to trigger the bug, so I have dropped it rather than add
equivalent functionality to the webserver in run-tests.py.
zackw added a commit to zackw/phantomjs that referenced this issue Aug 8, 2015
Also: Better naming convention for tests in test/regression/.

regression/pjs-12482.js should not use the harness, because it tests
phantom.exit.

set/690-ttf-crash/run.js is a regression test hiding off to the side;
include it.  NOTE: This test used to include a custom webserver process
that sent a bunch of response headers which were supposed to prevent
all caching.  Examination of issue ariya#10690 indicates this was *probably*
not necessary to trigger the bug, so I have dropped it rather than add
equivalent functionality to the webserver in run-tests.py.
zackw added a commit to zackw/phantomjs that referenced this issue Aug 10, 2015
Also: Better naming convention for tests in test/regression/.

regression/pjs-12482.js should not use the harness, because it tests
phantom.exit.

set/690-ttf-crash/run.js is a regression test hiding off to the side;
include it.  NOTE: This test used to include a custom webserver process
that sent a bunch of response headers which were supposed to prevent
all caching.  Examination of issue ariya#10690 indicates this was *probably*
not necessary to trigger the bug, so I have dropped it rather than add
equivalent functionality to the webserver in run-tests.py.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants