539775 - Fennec crashes loading nytimes.com

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Stuart Parmenter]

Goes away if I disable plugins

Comment 2

[Doug Turner (:dougt)]

#0  0x57420000 in ?? ()
#1  0x42ea8d44 in ns_if_addref<nsIPluginInstanceOwner*> (expr=<value optimized out>) at ../../../../dist/include/nsISupportsUtils.h:114
#2  0x42ea5c00 in nsNPAPIPluginInstance::GetOwner (this=0x4b485c00, aOwner=0xbe8a0754)
    at /home/dougt/mobile/mozilla-1.9.2/modules/plugin/base/src/nsNPAPIPluginInstance.cpp:2047
#3  0x42eaac38 in nsPluginHost::DoURLLoadSecurityCheck (this=<value optimized out>, aInstance=0x4b485c00, 
    aURL=0x4a76dac0 <Address 0x4a76dac0 out of bounds>) at /home/dougt/mobile/mozilla-1.9.2/modules/plugin/base/src/nsPluginHost.cpp:5717
#4  0x42eb1f6c in nsPluginHost::GetURLWithHeaders (this=Cannot access memory at address 0xbe8a0778
) at /home/dougt/mobile/mozilla-1.9.2/modules/plugin/base/src/nsPluginHost.cpp:2785
#5  0x42eb2128 in nsPluginHost::GetURL (this=0x467dd2f0, pluginInst=0xbe8a0754, url=0x4a76dac0 <Address 0x4a76dac0 out of bounds>, 
    target=0x467dd358 <Address 0x467dd358 out of bounds>, streamListener=Cannot access memory at address 0xbe8a0870
)
    at /home/dougt/mobile/mozilla-1.9.2/modules/plugin/base/src/nsPluginHost.cpp:2755
#6  0x42ea3b70 in MakeNewNPAPIStreamInternal (npp=<value optimized out>, relativeURL=Cannot access memory at address 0xbe8a086c
)
    at /home/dougt/mobile/mozilla-1.9.2/modules/plugin/base/src/nsNPAPIPlugin.cpp:912
#7  0x42ea3df4 in _geturlnotify (npp=Cannot access memory at address 0xbe8a08f0
) at /home/dougt/mobile/mozilla-1.9.2/modules/plugin/base/src/nsNPAPIPlugin.cpp:984
#8  0x48f2c1c0 in ?? () from /usr/lib/browser/plugins/libflashplayer.so
#9  0x48f2c1c0 in ?? () from /usr/lib/browser/plugins/libflashplayer.so



plugin instance (nsIPluginInstance) is created
plugin owner (nsIPluginInstanceOwner) owns the instance
instances has a back pointer to the plugin own

nsObjectFrame creates the plugin owner (nsIPluginInstanceOwner)

The crash happens because the Instance is deleted, and plugin still calls to it.

Which implies the plugin owner has gone away prematurely?

Comment 3

[Johnny Stenback (:jst)]

Would the fix for bug 533030 help here?

Comment 4

[Doug Turner (:dougt)]

I really wish it did. :(

What I see is that we call stop() on the plugin instance and that is successful (stop on the plugin is called).  We then tear down the plugin instance.  However, a few moments later, either invalidateRect or getvalue or geturl will be called on the destroyed plugin instance.

For example, here is a simple log of what I am seeing:

nsNPAPIPluginInstance->GetValue 0x47d62a00
nsNPAPIPluginInstance->GetValue 0x47d0b500
nsNPAPIPluginInstance->GetValue 0x47d0b500
nsNPAPIPluginInstance->Stop 0x47d0b500
  calling destory!
  returned ok!!
nsNPAPIPluginInstance->GetValue 0x47d0b500
...
Segmentation fault

Is the expectation that after Stop is called, the plugin should NOT call us back?

Comment 5

[Benjamin Smedberg]

Yes. Once NPP_Destroy has been called the plugin should not be making further calls, and we shouldn't be calling anything on that instance.

Comment 6

[Doug Turner (:dougt)]

another crash

#0  gfxTextRun::CompressedGlyph::IsClusterStart (this=0x3eb57d98) at ../../dist/include/gfxFont.h:1342
#1  0x430e1398 in gfxTextRun::SetPotentialLineBreaks (this=0x448796a0, aStart=3196811688, aLength=1149736608, aBreakBefore=0x433e9998 "\244h\16C@h\16C(\23\16CP\25\16C\360\61\16C\24b\16C", 
    aRefContext=0xbe8b795c) at /home/dougt/mobile/mozilla-1.9.2/gfx/thebes/src/gfxFont.cpp:1898
#2  0x430625c8 in ~nsCOMPtr_base (this=0xbe8b795c) at nsCOMPtr.cpp:81
#3  0x4283acb4 in ~nsCOMPtr (this=0x3eb57d98) at ../../dist/include/nsCOMPtr.h:469
#4  0x42ea5cb0 in nsNPAPIPluginInstance::InvalidateRect (this=<value optimized out>, invalidRect=0xbe8b79a8) at /home/dougt/mobile/mozilla-1.9.2/modules/plugin/base/src/nsNPAPIPluginInstance.cpp:1978
#5  0x42ea1fc8 in _invalidaterect (npp=<value optimized out>, invalidRect=0xbe8b79a8) at /home/dougt/mobile/mozilla-1.9.2/modules/plugin/base/src/nsNPAPIPlugin.cpp:1272
#6  0x4b22c458 in ?? () from /usr/lib/browser/plugins/libflashplayer.so


the nsNPAPIPluginInstance has since been destroyed.

Comment 7

[Doug Turner (:dougt)]

Attachment: workaround (leak instead of crash)

this is the worst thing I have thought about adding to the tree.  However, it seems to prevent us from crashing from the buggy flash.

Comment 8

[Aakash Desai [:aakashd]]

How bad is the leak ?

Comment 9

[Doug Turner (:dougt)]

better than a crash, but still crappy.

Comment 10

[Doug Turner (:dougt)]

Attachment: patch v.2

printfs need to be removed.

Comment 11

[Doug Turner (:dougt)]

Attachment: patch v.3

better macros.  printf's can be removed before pushing.

Comment 12

[Doug Turner (:dougt)]

* i am not convinced any longer that this provides enough protect against the flash crash *

There maybe be a bug in the logic.  Clearly if you allocate over 100 npp's you will crash, but we are crashing well under that limit now.

Comment 13

[Aakash Desai [:aakashd]]

I'm not seeing a crash on loading nytimes with 11/22 and later nightly builds. Is this fixed?

Comment 14

[Aakash Desai [:aakashd]]

verified FIXED on builds due to patches in bugs 540795 and 540843 :

Mozilla/5.0 (X11; U; Linux armv7l; Nokia N900; en-US; rv:1.9.2pre) Gecko/20100124 Namoroka/3.6pre Fennec/1.0pre