Bug 24592 – [GTK] Crash in FcPatternHash

• Component: WebKitGTK
• Status: RESOLVED FIXED

[Xan Lopez]

Wasn't running debug image, so only have this. Happens when closing a web view, can't reproduce at will:

Program received signal SIGSEGV, Segmentation fault.
[Switching to Thread 0xb56e7700 (LWP 29433)]
0xb5c8c501 in FcPatternHash () from /usr/lib/libfontconfig.so.1
(gdb) bt
#0  0xb5c8c501 in FcPatternHash () from /usr/lib/libfontconfig.so.1
#1  0xb7742d00 in WebCore::FontCache::purgeInactiveFontData () from /opt/gnome2/lib/libwebkit-1.0.so.2
#2  0xb7743dec in WebCore::FontCache::releaseFontData () from /opt/gnome2/lib/libwebkit-1.0.so.2
#3  0xb77472c9 in WebCore::FontFallbackList::releaseFontData () from /opt/gnome2/lib/libwebkit-1.0.so.2
#4  0xb7741b70 in WebCore::Font::~Font () from /opt/gnome2/lib/libwebkit-1.0.so.2
#5  0xb7840228 in WebCore::StyleInheritedData::~StyleInheritedData () from /opt/gnome2/lib/libwebkit-1.0.so.2
#6  0xb783aa20 in WebCore::RenderStyle::~RenderStyle () from /opt/gnome2/lib/libwebkit-1.0.so.2
#7  0xb77f84b8 in WebCore::RenderObject::~RenderObject () from /opt/gnome2/lib/libwebkit-1.0.so.2
#8  0xb77bd875 in WebCore::RenderBoxModelObject::~RenderBoxModelObject () from /opt/gnome2/lib/libwebkit-1.0.so.2
#9  0xb77b56e5 in WebCore::RenderBox::~RenderBox () from /opt/gnome2/lib/libwebkit-1.0.so.2
#10 0xb77a4d2a in WebCore::RenderBlock::~RenderBlock () from /opt/gnome2/lib/libwebkit-1.0.so.2
#11 0xb77f3753 in WebCore::RenderObject::arenaDelete () from /opt/gnome2/lib/libwebkit-1.0.so.2
#12 0xb77f3843 in WebCore::RenderObject::destroy () from /opt/gnome2/lib/libwebkit-1.0.so.2
#13 0xb77bd7c1 in WebCore::RenderBoxModelObject::destroy () from /opt/gnome2/lib/libwebkit-1.0.so.2
#14 0xb77b6cd2 in WebCore::RenderBox::destroy () from /opt/gnome2/lib/libwebkit-1.0.so.2
#15 0xb77a129d in WebCore::RenderBlock::destroy () from /opt/gnome2/lib/libwebkit-1.0.so.2
#16 0xb754d369 in WebCore::Node::detach () from /opt/gnome2/lib/libwebkit-1.0.so.2
#17 0xb7518a4e in WebCore::ContainerNode::detach () from /opt/gnome2/lib/libwebkit-1.0.so.2
#18 0xb753dbd3 in WebCore::Element::detach () from /opt/gnome2/lib/libwebkit-1.0.so.2
#19 0xb7518a3b in WebCore::ContainerNode::detach () from /opt/gnome2/lib/libwebkit-1.0.so.2
#20 0xb753dbd3 in WebCore::Element::detach () from /opt/gnome2/lib/libwebkit-1.0.so.2
#21 0xb7518a3b in WebCore::ContainerNode::detach () from /opt/gnome2/lib/libwebkit-1.0.so.2
#22 0xb753dbd3 in WebCore::Element::detach () from /opt/gnome2/lib/libwebkit-1.0.so.2
#23 0xb7518a3b in WebCore::ContainerNode::detach () from /opt/gnome2/lib/libwebkit-1.0.so.2
#24 0xb753dbd3 in WebCore::Element::detach () from /opt/gnome2/lib/libwebkit-1.0.so.2
#25 0xb7518a3b in WebCore::ContainerNode::detach () from /opt/gnome2/lib/libwebkit-1.0.so.2
#26 0xb753dbd3 in WebCore::Element::detach () from /opt/gnome2/lib/libwebkit-1.0.so.2
#27 0xb7518a3b in WebCore::ContainerNode::detach () from /opt/gnome2/lib/libwebkit-1.0.so.2
#28 0xb7520d05 in WebCore::Document::detach () from /opt/gnome2/lib/libwebkit-1.0.so.2
#29 0xb76f9884 in WebCore::Frame::setView () from /opt/gnome2/lib/libwebkit-1.0.so.2
#30 0xb76f990f in WebCore::Frame::createView () from /opt/gnome2/lib/libwebkit-1.0.so.2
#31 0xb7434da5 in WebKit::FrameLoaderClient::transitionToCommittedForNewPage ()
   from /opt/gnome2/lib/libwebkit-1.0.so.2
#32 0xb76a4b46 in WebCore::FrameLoader::transitionToCommitted () from /opt/gnome2/lib/libwebkit-1.0.so.2
#33 0xb76a545e in WebCore::FrameLoader::commitProvisionalLoad () from /opt/gnome2/lib/libwebkit-1.0.so.2
#34 0xb768448d in WebCore::DocumentLoader::commitIfReady () from /opt/gnome2/lib/libwebkit-1.0.so.2
#35 0xb7684916 in WebCore::DocumentLoader::commitLoad () from /opt/gnome2/lib/libwebkit-1.0.so.2
---Type <return> to continue, or q <return> to quit---
#36 0xb7693175 in WebCore::FrameLoader::receivedData () from /opt/gnome2/lib/libwebkit-1.0.so.2
#37 0xb76ac1b6 in WebCore::MainResourceLoader::addData () from /opt/gnome2/lib/libwebkit-1.0.so.2
#38 0xb76b4139 in WebCore::ResourceLoader::didReceiveData () from /opt/gnome2/lib/libwebkit-1.0.so.2
#39 0xb76ac401 in WebCore::MainResourceLoader::didReceiveData () from /opt/gnome2/lib/libwebkit-1.0.so.2
#40 0xb76b3a48 in WebCore::ResourceLoader::didReceiveData () from /opt/gnome2/lib/libwebkit-1.0.so.2
#41 0xb784c755 in WebCore::gotChunkCallback () from /opt/gnome2/lib/libwebkit-1.0.so.2
#42 0xb5bb852a in IA__g_cclosure_marshal_VOID__BOXED (closure=0x9424910, return_value=0x0, n_param_values=2, 
    param_values=0x8c00cf0, invocation_hint=0xbfc83dfc, marshal_data=0x93ef068) at gmarshal.c:566
#43 0xb5ba9fdb in IA__g_closure_invoke (closure=0x9424910, return_value=0x0, n_param_values=2, 
    param_values=0x8c00cf0, invocation_hint=0xbfc83dfc) at gclosure.c:767
#44 0xb5bc16e7 in signal_emit_unlocked_R (node=0x8a4ae28, detail=0, instance=0x93ef068, emission_return=0x0, 
    instance_and_params=0x8c00cf0) at gsignal.c:3244
#45 0xb5bc2d5b in IA__g_signal_emit_valist (instance=0x93ef068, signal_id=374, detail=0, 
    var_args=0xbfc83fa0 "��\026�\034q\031�\030`ȿ%�\026�h�>\t8\236Y\b") at gsignal.c:2977
#46 0xb5bc3206 in IA__g_signal_emit (instance=0x93ef068, signal_id=374, detail=0) at gsignal.c:3034
#47 0xb616a316 in soup_message_got_chunk (msg=0x93ef068, chunk=0x8599e38) at soup-message.c:775
#48 0xb616f325 in read_body_chunk (msg=0x93ef068) at soup-message-io.c:313
#49 0xb616fa15 in io_read (sock=0x9b5cc18, msg=0x93ef068) at soup-message-io.c:785
#50 0xb5bb7e84 in IA__g_cclosure_marshal_VOID__VOID (closure=0x8e8a508, return_value=0x0, n_param_values=1, 
    param_values=0x8599c80, invocation_hint=0xbfc8620c, marshal_data=0xb616f6e0) at gmarshal.c:77
#51 0xb5ba9fdb in IA__g_closure_invoke (closure=0x8e8a508, return_value=0x0, n_param_values=1, 
    param_values=0x8599c80, invocation_hint=0xbfc8620c) at gclosure.c:767
#52 0xb5bc16e7 in signal_emit_unlocked_R (node=0x8a169c0, detail=0, instance=0x9b5cc18, emission_return=0x0, 
    instance_and_params=0x8599c80) at gsignal.c:3244
#53 0xb5bc2d5b in IA__g_signal_emit_valist (instance=0x9b5cc18, signal_id=382, detail=0, 
    var_args=0xbfc863ac "�B���B��h\035\034\n�cȿ����`�\032\n\001") at gsignal.c:2977
#54 0xb5bc3206 in IA__g_signal_emit (instance=0x9b5cc18, signal_id=382, detail=0) at gsignal.c:3034
#55 0xb617a402 in socket_read_watch (chan=0xa1aa460, cond=<value optimized out>, user_data=0x9b5cc18)
    at soup-socket.c:1074
#56 0xb5afa2bd in g_io_unix_dispatch (source=0xa1c1d68, callback=0xb617a3b0 <socket_read_watch>, 
    user_data=0x9b5cc18) at giounix.c:162
#57 0xb5ac30c8 in IA__g_main_context_dispatch (context=0x835e838) at gmain.c:1814
#58 0xb5ac662b in g_main_context_iterate (context=0x835e838, block=1, dispatch=1, self=0x8336480) at gmain.c:2448
#59 0xb5ac6afa in IA__g_main_loop_run (loop=0x83899d0) at gmain.c:2656
#60 0xb628b409 in IA__gtk_main () at gtkmain.c:1205
#61 0x08048c86 in main (argc=Cannot access memory at address 0x4f0682
) at ../../../src/ephy-main.c:781
(gdb) r

[Xan Lopez]

Just had this again, and it's crashing when trying to access the first pattern element in here:

FcChar32
FcPatternHash (const FcPattern *p)
{
    int		i;
    FcChar32	h = 0;
    FcPatternElt    *pe = FcPatternElts(p);

    for (i = 0; i < p->num; i++)
    {
	h = (((h << 1) | (h >> 31)) ^ 
	     pe[i].object ^ // <--- crash
	     FcValueListHash (FcPatternEltValues(&pe[i])));
    }

so it seems we are passing an already deleted pattern to the function.

[Xan Lopez]

(gdb) bt
#0  IA__FcPatternHash (p=0x95c3468) at fcpat.c:432
#1  0xb6ad668e in WebCore::FontPlatformData::hash (this=0x931f188) at ../../WebCore/platform/graphics/gtk/FontPlatformData.h:91
#2  0xb6ad66d3 in WebCore::FontDataCacheKeyHash::hash (platformData=@0x931f188) at ../../WebCore/platform/graphics/FontCache.cpp:223
#3  0xb6ad66e6 in WTF::IdentityHashTranslator<WebCore::FontPlatformData, std::pair<WebCore::FontPlatformData, std::pair<WebCore::SimpleFontData*, unsigned int> >, WebCore::FontDataCacheKeyHash>::hash (key=@0x931f188) at ../../JavaScriptCore/wtf/HashTable.h:277
#4  0xb6ad6a1d in WTF::HashTable<WebCore::FontPlatformData, std::pair<WebCore::FontPlatformData, std::pair<WebCore::SimpleFontData*, unsigned int> >, WTF::PairFirstExtractor<std::pair<WebCore::FontPlatformData, std::pair<WebCore::SimpleFontData*, unsigned int> > >, WebCore::FontDataCacheKeyHash, WTF::PairHashTraits<WebCore::FontDataCacheKeyTraits, WTF::HashTraits<std::pair<WebCore::SimpleFontData*, unsigned int> > >, WebCore::FontDataCacheKeyTraits>::lookup<WebCore::FontPlatformData, WTF::IdentityHashTranslator<WebCore::FontPlatformData, std::pair<WebCore::FontPlatformData, std::pair<WebCore::SimpleFontData*, unsigned int> >, WebCore::FontDataCacheKeyHash> > (this=0x8eea850, key=@0x931f188)
    at ../../JavaScriptCore/wtf/HashTable.h:474
#5  0xb6ad6bf1 in WTF::HashTable<WebCore::FontPlatformData, std::pair<WebCore::FontPlatformData, std::pair<WebCore::SimpleFontData*, unsigned int> >, WTF::PairFirstExtractor<std::pair<WebCore::FontPlatformData, std::pair<WebCore::SimpleFontData*, unsigned int> > >, WebCore::FontDataCacheKeyHash, WTF::PairHashTraits<WebCore::FontDataCacheKeyTraits, WTF::HashTraits<std::pair<WebCore::SimpleFontData*, unsigned int> > >, WebCore::FontDataCacheKeyTraits>::contains<WebCore::FontPlatformData, WTF::IdentityHashTranslator<WebCore::FontPlatformData, std::pair<WebCore::FontPlatformData, std::pair<WebCore::SimpleFontData*, unsigned int> >, WebCore::FontDataCacheKeyHash> > (this=0x8eea850, key=@0x931f188)
    at ../../JavaScriptCore/wtf/HashTable.h:794
#6  0xb6ad6c22 in WTF::HashTable<WebCore::FontPlatformData, std::pair<WebCore::FontPlatformData, std::pair<WebCore::SimpleFontData*, unsigned int> >, WTF::PairFirstExtractor<std::pair<WebCore::FontPlatformData, std::pair<WebCore::SimpleFontData*, unsigned int> > >, WebCore::FontDataCacheKeyHash, WTF::PairHashTraits<WebCore::FontDataCacheKeyTraits, WTF::HashTraits<std::pair<WebCore::SimpleFontData*, unsigned int> > >, WebCore::FontDataCacheKeyTraits>::contains (this=0x8eea850, key=@0x931f188) at ../../JavaScriptCore/wtf/HashTable.h:325
#7  0xb6ad6c40 in WTF::HashMap<WebCore::FontPlatformData, std::pair<WebCore::SimpleFontData*, unsigned int>, WebCore::FontDataCacheKeyHash, WebCore::FontDataCacheKeyTraits, WTF::HashTraits<std::pair<WebCore::SimpleFontData*, unsigned int> > >::contains (this=0x8eea850, key=@0x931f188)
    at ../../JavaScriptCore/wtf/HashMap.h:173
#8  0xb6ace352 in WebCore::FontCache::purgeInactiveFontData (this=0x8d7c2f0, count=21) at ../../WebCore/platform/graphics/FontCache.cpp:330
#9  0xb6ace865 in WebCore::FontCache::releaseFontData (this=0x8d7c2f0, fontData=0xab93518) at ../../WebCore/platform/graphics/FontCache.cpp:295
#10 0xb6ad8e17 in WebCore::FontFallbackList::releaseFontData (this=0xb3c2270) at ../../WebCore/platform/graphics/FontFallbackList.cpp:64
#11 0xb6acc793 in ~FontFallbackList (this=0xb3c2270) at ../../WebCore/platform/graphics/FontFallbackList.h:46
#12 0xb6acc7f3 in WTF::RefCounted<WebCore::FontFallbackList>::deref (this=0xb3c2270) at ../../JavaScriptCore/wtf/RefCounted.h:94
#13 0xb6acc87b in ~RefPtr (this=0xc1f4e94) at ../../JavaScriptCore/wtf/RefPtr.h:50
#14 0xb6acb404 in ~Font (this=0xc1f4e80) at ../../WebCore/platform/graphics/Font.cpp:118
#15 0xb6c2b898 in ~StyleInheritedData (this=0xc1f4e68) at ../../WebCore/rendering/style/StyleInheritedData.cpp:46
#16 0xb67e4ed1 in WTF::RefCounted<WebCore::StyleInheritedData>::deref (this=0xc1f4e68) at ../../JavaScriptCore/wtf/RefCounted.h:94
#17 0xb6c2798d in ~RefPtr (this=0xad165b8) at ../../JavaScriptCore/wtf/RefPtr.h:50
#18 0xb6c279a1 in ~DataRef (this=0xad165b8) at ../../WebCore/rendering/style/DataRef.h:31
#19 0xb6c25984 in ~RenderStyle (this=0xad16588) at ../../WebCore/rendering/style/RenderStyle.cpp:163
#20 0xb6759af5 in WTF::RefCounted<WebCore::RenderStyle>::deref (this=0xad16588) at ../../JavaScriptCore/wtf/RefCounted.h:94
---Type <return> to continue, or q <return> to quit---
#21 0xb6759bbb in ~RefPtr (this=0xb194610) at ../../JavaScriptCore/wtf/RefPtr.h:50
#22 0xb6bccb74 in ~RenderObject (this=0xb19460c) at ../../WebCore/rendering/RenderObject.cpp:215
#23 0xb6b8a686 in ~RenderBoxModelObject (this=0xb19460c) at ../../WebCore/rendering/RenderBoxModelObject.cpp:56
#24 0xb6ba4a73 in ~RenderInline (this=0xb19460c) at ../../WebCore/rendering/RenderInline.cpp:56
#25 0xb6bc5d9e in WebCore::RenderObject::arenaDelete (this=0xb19460c, arena=0xc0155c8, base=0xb19460c)
    at ../../WebCore/rendering/RenderObject.cpp:1861
#26 0xb6bc5f5e in WebCore::RenderObject::destroy (this=0xb19460c) at ../../WebCore/rendering/RenderObject.cpp:1834
#27 0xb6b8a31f in WebCore::RenderBoxModelObject::destroy (this=0xb19460c) at ../../WebCore/rendering/RenderBoxModelObject.cpp:74
#28 0xb6ba4a33 in WebCore::RenderInline::destroy (this=0xb19460c) at ../../WebCore/rendering/RenderInline.cpp:93
#29 0xb68695e6 in WebCore::Node::detach (this=0xb1d00e48) at ../../WebCore/dom/Node.cpp:1103
#30 0xb680c413 in WebCore::ContainerNode::detach (this=0xb1d00e48) at ../../WebCore/dom/ContainerNode.cpp:599
#31 0xb684cd20 in WebCore::Element::detach (this=0xb1d00e48) at ../../WebCore/dom/Element.cpp:720
#32 0xb680c3e1 in WebCore::ContainerNode::detach (this=0xb1d009c0) at ../../WebCore/dom/ContainerNode.cpp:597
#33 0xb684cd20 in WebCore::Element::detach (this=0xb1d009c0) at ../../WebCore/dom/Element.cpp:720
#34 0xb680c3e1 in WebCore::ContainerNode::detach (this=0xb1308008) at ../../WebCore/dom/ContainerNode.cpp:597
#35 0xb684cd20 in WebCore::Element::detach (this=0xb1308008) at ../../WebCore/dom/Element.cpp:720
#36 0xb680c3e1 in WebCore::ContainerNode::detach (this=0xb13647e8) at ../../WebCore/dom/ContainerNode.cpp:597
#37 0xb684cd20 in WebCore::Element::detach (this=0xb13647e8) at ../../WebCore/dom/Element.cpp:720
#38 0xb680c3e1 in WebCore::ContainerNode::detach (this=0xb136c298) at ../../WebCore/dom/ContainerNode.cpp:597
#39 0xb684cd20 in WebCore::Element::detach (this=0xb136c298) at ../../WebCore/dom/Element.cpp:720
#40 0xb680c3e1 in WebCore::ContainerNode::detach (this=0xab22478) at ../../WebCore/dom/ContainerNode.cpp:597
#41 0xb684cd20 in WebCore::Element::detach (this=0xab22478) at ../../WebCore/dom/Element.cpp:720
#42 0xb680c3e1 in WebCore::ContainerNode::detach (this=0xbdfd540) at ../../WebCore/dom/ContainerNode.cpp:597
#43 0xb684cd20 in WebCore::Element::detach (this=0xbdfd540) at ../../WebCore/dom/Element.cpp:720
#44 0xb680c3e1 in WebCore::ContainerNode::detach (this=0xb13be98) at ../../WebCore/dom/ContainerNode.cpp:597
#45 0xb684cd20 in WebCore::Element::detach (this=0xb13be98) at ../../WebCore/dom/Element.cpp:720
#46 0xb680c3e1 in WebCore::ContainerNode::detach (this=0xc812448) at ../../WebCore/dom/ContainerNode.cpp:597
#47 0xb681b946 in WebCore::Document::detach (this=0xc812448) at ../../WebCore/dom/Document.cpp:1310
#48 0xb6a6bf9e in WebCore::Frame::setView (this=0x9299900, view=0x0) at ../../WebCore/page/Frame.cpp:232
#49 0xb6a6c133 in WebCore::Frame::createView (this=0x9299900, viewportSize=@0xbfeb02a4, backgroundColor=@0xbfeb029c, transparent=false, 
    fixedLayoutSize=@0xbfeb0294, useFixedLayout=false, horizontalScrollbarMode=WebCore::ScrollbarAuto, 
    verticalScrollbarMode=WebCore::ScrollbarAuto) at ../../WebCore/page/Frame.cpp:1737
#50 0xb66ae11d in WebKit::FrameLoaderClient::transitionToCommittedForNewPage (this=0x9e79870)
    at ../../WebKit/gtk/WebCoreSupport/FrameLoaderClientGtk.cpp:918
#51 0xb6a062c9 in WebCore::FrameLoader::transitionToCommitted (this=0x929992c, cachedPage={m_ptr = 0xbfeb0394})
    at ../../WebCore/loader/FrameLoader.cpp:2901
#52 0xb6a06e19 in WebCore::FrameLoader::commitProvisionalLoad (this=0x929992c, prpCachedPage={m_ptr = 0xbfeb0490})
---Type <return> to continue, or q <return> to quit---
    at ../../WebCore/loader/FrameLoader.cpp:2772
#53 0xb69df644 in WebCore::DocumentLoader::commitIfReady (this=0xc5b51c8) at ../../WebCore/loader/DocumentLoader.cpp:339
#54 0xb69e1181 in WebCore::DocumentLoader::commitLoad (this=0xc5b51c8, 
    data=0xbfeb09b0 "<!DOCTYPE html PUBLIC \"-//W3C//DTD XHTML 1.0 Strict//EN\" \"http://www.w3.org/TR/xhtml1/DTD/xhtml1-strict.dtd\">\n<html xmlns='http://www.w3.org/1999/xhtml' xmlns:b='http://www.google.com/2005/gml/b' xmln"..., length=1418) at ../../WebCore/loader/DocumentLoader.cpp:359
#55 0xb69e121e in WebCore::DocumentLoader::receivedData (this=0xc5b51c8, 
    data=0xbfeb09b0 "<!DOCTYPE html PUBLIC \"-//W3C//DTD XHTML 1.0 Strict//EN\" \"http://www.w3.org/TR/xhtml1/DTD/xhtml1-strict.dtd\">\n<html xmlns='http://www.w3.org/1999/xhtml' xmlns:b='http://www.google.com/2005/gml/b' xmln"..., length=1418) at ../../WebCore/loader/DocumentLoader.cpp:373
#56 0xb69f9b59 in WebCore::FrameLoader::receivedData (this=0x929992c, 
    data=0xbfeb09b0 "<!DOCTYPE html PUBLIC \"-//W3C//DTD XHTML 1.0 Strict//EN\" \"http://www.w3.org/TR/xhtml1/DTD/xhtml1-strict.dtd\">\n<html xmlns='http://www.w3.org/1999/xhtml' xmlns:b='http://www.google.com/2005/gml/b' xmln"..., length=1418) at ../../WebCore/loader/FrameLoader.cpp:2423
#57 0xb6a0fa9e in WebCore::MainResourceLoader::addData (this=0xc2cc4c8, 
    data=0xbfeb09b0 "<!DOCTYPE html PUBLIC \"-//W3C//DTD XHTML 1.0 Strict//EN\" \"http://www.w3.org/TR/xhtml1/DTD/xhtml1-strict.dtd\">\n<html xmlns='http://www.w3.org/1999/xhtml' xmlns:b='http://www.google.com/2005/gml/b' xmln"..., length=1418, allAtOnce=false)
    at ../../WebCore/loader/MainResourceLoader.cpp:146
#58 0xb6a15ea7 in WebCore::ResourceLoader::didReceiveData (this=0xc2cc4c8, 
    data=0xbfeb09b0 "<!DOCTYPE html PUBLIC \"-//W3C//DTD XHTML 1.0 Strict//EN\" \"http://www.w3.org/TR/xhtml1/DTD/xhtml1-strict.dtd\">\n<html xmlns='http://www.w3.org/1999/xhtml' xmlns:b='http://www.google.com/2005/gml/b' xmln"..., length=1418, lengthReceived=0, allAtOnce=false)
    at ../../WebCore/loader/ResourceLoader.cpp:257
#59 0xb6a0eb4e in WebCore::MainResourceLoader::didReceiveData (this=0xc2cc4c8, 
    data=0xbfeb09b0 "<!DOCTYPE html PUBLIC \"-//W3C//DTD XHTML 1.0 Strict//EN\" \"http://www.w3.org/TR/xhtml1/DTD/xhtml1-strict.dtd\">\n<html xmlns='http://www.w3.org/1999/xhtml' xmlns:b='http://www.google.com/2005/gml/b' xmln"..., length=1418, lengthReceived=0, allAtOnce=false)
    at ../../WebCore/loader/MainResourceLoader.cpp:347
#60 0xb6a1512a in WebCore::ResourceLoader::didReceiveData (this=0xc2cc4c8, 
    data=0xbfeb09b0 "<!DOCTYPE html PUBLIC \"-//W3C//DTD XHTML 1.0 Strict//EN\" \"http://www.w3.org/TR/xhtml1/DTD/xhtml1-strict.dtd\">\n<html xmlns='http://www.w3.org/1999/xhtml' xmlns:b='http://www.google.com/2005/gml/b' xmln"..., length=1418, lengthReceived=0)
    at ../../WebCore/loader/ResourceLoader.cpp:411
#61 0xb6c3a0f9 in gotChunkCallback (msg=0xbd61988, chunk=0xbb85898, data=0xa8686f0)
    at ../../WebCore/platform/network/soup/ResourceHandleSoup.cpp:252
#62 0xb4daf52a in IA__g_cclosure_marshal_VOID__BOXED (closure=0xbc822a0, return_value=0x0, n_param_values=2, param_values=0xc64a200, 
    invocation_hint=0xbfeb07dc, marshal_data=0xb6c39f43) at gmarshal.c:566
#63 0xb4da0fdb in IA__g_closure_invoke (closure=0xbc822a0, return_value=0x0, n_param_values=2, param_values=0xc64a200, invocation_hint=0xbfeb07dc)
    at gclosure.c:767
#64 0xb4db86e7 in signal_emit_unlocked_R (node=0x8e7d620, detail=0, instance=0xbd61988, emission_return=0x0, instance_and_params=0xc64a200)
    at gsignal.c:3244
#65 0xb4db9d5b in IA__g_signal_emit_valist (instance=0xbd61988, signal_id=374, detail=0, 
    var_args=0xbfeb0980 "�B5�\034\0218��)��%\2235�\210\031�\v\230X�\v") at gsignal.c:2977
---Type <return> to continue, or q <return> to quit---
#66 0xb4dba206 in IA__g_signal_emit (instance=0xbd61988, signal_id=374, detail=0) at gsignal.c:3034
#67 0xb5354316 in soup_message_got_chunk (msg=0xbd61988, chunk=0xbb85898) at soup-message.c:830
#68 0xb5359325 in read_body_chunk (msg=0xbd61988) at soup-message-io.c:313
#69 0xb53599ad in io_read (sock=0x9462e48, msg=0xbd61988) at soup-message-io.c:758
#70 0xb4daee84 in IA__g_cclosure_marshal_VOID__VOID (closure=0x973fd20, return_value=0x0, n_param_values=1, param_values=0x8a87dc0, 
    invocation_hint=0xbfeb2bec, marshal_data=0xb53596e0) at gmarshal.c:77
#71 0xb4da0fdb in IA__g_closure_invoke (closure=0x973fd20, return_value=0x0, n_param_values=1, param_values=0x8a87dc0, invocation_hint=0xbfeb2bec)
    at gclosure.c:767
#72 0xb4db86e7 in signal_emit_unlocked_R (node=0x8e2b278, detail=0, instance=0x9462e48, emission_return=0x0, instance_and_params=0x8a87dc0)
    at gsignal.c:3244
#73 0xb4db9d5b in IA__g_signal_emit_valist (instance=0x9462e48, signal_id=382, detail=0, 
    var_args=0xbfeb2d8c "��ִ��ִ`�\177\v�-뿽\022ϴ\030\2073\f\001") at gsignal.c:2977
#74 0xb4dba206 in IA__g_signal_emit (instance=0x9462e48, signal_id=382, detail=0) at gsignal.c:3034
#75 0xb5364402 in socket_read_watch (chan=0xc338718, cond=<value optimized out>, user_data=0x9462e48) at soup-socket.c:1116
#76 0xb4cf12bd in g_io_unix_dispatch (source=0xb7fa560, callback=0xb53643b0 <socket_read_watch>, user_data=0x9462e48) at giounix.c:162
#77 0xb4cba0c8 in IA__g_main_context_dispatch (context=0x8768880) at gmain.c:1814
#78 0xb4cbd62b in g_main_context_iterate (context=0x8768880, block=1, dispatch=1, self=0x87404b8) at gmain.c:2448
#79 0xb4cbdafa in IA__g_main_loop_run (loop=0x87959c8) at gmain.c:2656
#80 0xb545bf29 in IA__gtk_main () at gtkmain.c:1205
#81 0x08048c86 in main (argc=Cannot access memory at address 0x157b6e68
) at ../../../src/ephy-main.c:781
Current language:  auto; currently c

[Xan Lopez]

Created attachment 28653 [details]
gtkfonts.patch

This patch makes FontPlatformDataGtk free the memory it allocates in its own destructor, instead of having SimpleFontDataGtk free it in platformDestroy. Also, since the class has reference counted members we need a copy constructor and '=' operator to properly track those references when assigning and copying objects. With these two changes I can't reproduce the crash anymore.

Holger said he was concerned this might be a performance regression, but: other ports have objects that they also have to copy in their FontPlatformData* classes, so I don't see how this is that different. Besides, what this does is to add two extra refcounts per copy, it does not do actual copies of the fc pattern or the cairo fonts. That being said, there might be better ways to handle this, so I'm totally open to comments :)

[Holger Freyther]

I'm convinced this is the proper change. You are absolutely right in terms of information hiding that FontPlatformData should own these things.

[Holger Freyther]

Comment on attachment 28653 [details]
gtkfonts.patch

>         * platform/graphics/gtk/FontPlatformData.h:
>         * platform/graphics/gtk/FontPlatformDataGtk.cpp:


FontPlatformDataPango.cpp needs some love too?


> +FontPlatformData& FontPlatformData::operator=(const FontPlatformData& other)
> +{
> +    m_size = other.m_size;
> +    m_syntheticBold = other.m_syntheticBold;
> +    m_syntheticOblique = other.m_syntheticOblique;
> +    if (other.m_scaledFont)
> +        m_scaledFont = cairo_scaled_font_reference (other.m_scaledFont);
> +    else
> +        m_scaledFont = 0;
> +
> +    if (other.m_pattern)
> +        FcPatternReference(other.m_pattern);
> +    m_pattern = other.m_pattern;
> +
> +    // This will be re-created on demand.
> +    m_fallbacks = 0;

This is not correct. FontPlatformData might already have a properly initialized m_scaledFont and m_pattern. This means the normal flow of things are. Ref the new stuff, unref the old stuff, copy over the pointers. This makes sure you don't lead and guards you against self assignment (fontData = fontData). And in case of the self assignment you reset m_fallbacks without freeing it...

A incomplete pointer: http://www.codingstandard.com/HICPPCM/High_Integrity_CPP_Rule_3.1.5.html


> +FontPlatformData::FontPlatformData(const FontPlatformData& other)
> +{
       *this = other;

to avoid the code duplication.

[Xan Lopez]

Created attachment 28682 [details]
gtkfontsv2.patch

Address Holger review.

[Xan Lopez]

I think I've fixed all your comments (thanks, great review), save for the Pango thing. I think Alp has said that the FC backend does everything the Pango one did, and that it's pointless to use it/maintain it now. If he can confirm this I think we should remove it (?), otherwise I'll make another patch to apply the same fix.

[Xan Lopez]

+    if (other.m_scaledFont)
+        cairo_scaled_font_reference (other.m_scaledFont);

Meh, bad space. I'll fix that when landing if the patch is OK :)

[Holger Freyther]

Comment on attachment 28682 [details]
gtkfontsv2.patch


> +    // Do this first, since we need to catch self assignment.
> +    if (m_fallbacks && !(*this == other)) {

okay, we don't want to go through the operator== of this class but really compare the pointers of this and &other. You might want to consider having if (this == &other)\ return *this; at the top but this would optimize for the unlikely case... at least change it to compare the pointers as discussed on irc.


> -    if (m_font.m_pattern && ((FcPattern*)-1 != m_font.m_pattern)) {

we don't do the -1 check in the assignment operator and it seems fine. So let us schedule this bit for removal too (later patch or might be proven wrong if the assignment is crashing).



regarding the pango backend. we should update it too, it is needed for users on OSX (native/framework) and win32...

[Xan Lopez]

Created attachment 28690 [details]
pangofonts.patch

Same fix for the pango backend.

[Xan Lopez]

Created attachment 28691 [details]
style.patch

Style fix which I forgot to apply in the first patch.

[Holger Freyther]

Comment on attachment 28690 [details]
pangofonts.patch


> -  PangoFontMap* FontPlatformData::m_fontMap = 0;
> -  GHashTable* FontPlatformData::m_hashTable = 0;
> +PangoFontMap* FontPlatformData::m_fontMap = 0;
> +GHashTable* FontPlatformData::m_hashTable = 0;

maybe move this to the style patch?


>  
>  FontPlatformData::FontPlatformData(const FontDescription& fontDescription, const AtomicString& familyName)
>      : m_context(0)
> @@ -193,7 +194,20 @@ bool FontPlatformData::init()
>  
>  FontPlatformData::~FontPlatformData()
>  {
> -    // Destroy takes place in FontData::platformDestroy().
> +    if (m_font && m_font != reinterpret_cast<PangoFont*>(-1)) {

probably the same as with the previous patch. (-1)

[Holger Freyther]

Comment on attachment 28691 [details]
style.patch


>      if (other.m_scaledFont)
> -        cairo_scaled_font_reference (other.m_scaledFont);
> +        cairo_scaled_font_reference(other.m_scaledFont);

damn, you tricked me. Sorry for not noticing. :)

[Xan Lopez]

All patches landed (r41762, r41767 and r41768), closing as FIXED.