706381 - Java related crash with deleted pointer in esx

Status: RESOLVED FIXED

(Core Graveyard :: Plug-ins, defect)

Description

[Bob Clary [:bc] (inactive)]

Attachment: crash report

0. enable Java(TM) Platform SE 6 U29
1. enable popups
2. disable malware detector
3. http://www.sheru100.in/view/index585.html
4. shutdown

http://cpm1.jmxy.com:899/cpm/tg_href.aspx?tg_d=www.ads80.com will be loaded which is reported as a malware attack site.

Nightly/11 Windows 7
bp-f1059123-a28f-499c-9003-5ed772111129
Crash Report [@ mozalloc_abort(char const* const) | NS_DebugBreak_P | mozilla::plugins::PluginModuleChild::ShouldContinueFromReplyTimeout() ] 

See also bug 681385

Automation hit this with ecx pointing to deleted memory. I wasn't able to reproduce locally though.

Operating system: Windows NT
                  6.1.7601 Service Pack 1
CPU: x86
     GenuineIntel family 6 model 44 stepping 2
     2 CPUs

Crash reason:  EXCEPTION_ACCESS_VIOLATION_WRITE
Crash address: 0xffffffffddddde1d

Thread 0 (crashed)
 0  xul.dll!nsNPAPIPluginInstance::InvalidateOwner() [nsNPAPIPluginInstance.cpp : 1431 + 0x3]
    eip = 0x6f9ed2ea   esp = 0x0031b128   ebp = 0x0031b12c   ebx = 0x00000001
    esi = 0x04ae00e8   edi = 0xffffff87   eax = 0xdddddddd   ecx = 0xdddddddd
    edx = 0x00000001   efl = 0x00210246
    Found by: given as instruction pointer in context
 1  xul.dll!nsDummyJavaPluginOwner::Destroy() [nsGlobalWindow.cpp : 483 + 0x11]
    eip = 0x6f28450a   esp = 0x0031b134   ebp = 0x0031b138
    Found by: call frame info
 2  xul.dll!nsGlobalWindow::FreeInnerObjects(int) [nsGlobalWindow.cpp : 1311 + 0x14]
    eip = 0x6f2874ff   esp = 0x0031b140   ebp = 0x0031b168
    Found by: call frame info
 3  xul.dll!nsGlobalWindow::SetDocShell(nsIDocShell *) [nsGlobalWindow.cpp : 2397 + 0x10]
    eip = 0x6f28bde4   esp = 0x0031b170   ebp = 0x0031b1c8
    Found by: call frame info
 4  xul.dll!nsDocShell::Destroy() [nsDocShell.cpp : 4611 + 0x1c]
    eip = 0x6f82852a   esp = 0x0031b1d0   ebp = 0x0031b22c
    Found by: call frame info
 5  xul.dll!nsFrameLoader::Finalize() [nsFrameLoader.cpp : 573 + 0x18]
    eip = 0x6efbc3a1   esp = 0x0031b234   ebp = 0x0031b248
    Found by: call frame info

Comment 1

[Bob Clary [:bc] (inactive)]

Attachment: log

Comment 2

[Daniel Veditz [:dveditz]]

Can we capture the testcase? If the site is already flagged as a malware site the contents might go away at any time.

Comment 3

[Bob Clary [:bc] (inactive)]

I'll try again. It was late and the attempts I made using wget didn't seem to catch anything useful.

Comment 4

[Johnny Stenback (:jst)]

Attachment: Possible fix.

It's unclear exactly how this crash happens, but I did find this inconsistency where we can create a dummy java plugin (which we create when accessing window.java n friends) and then navigate switch documents in the window w/o creating a new inner. In that case we end up with a dummy java plugin whose owner has a reference to the old document rather than the new one. This fixes that case, but I have not had a chance to test whether this fixes the problem here.

Bob, any chance you can test this out? I can spin a try build if that helps.

Comment 5

[Bob Clary [:bc] (inactive)]

dveditz was right that this might be fleeting. I couldn't get anything to reproduce except for some really high cpu usage. I can retry again. A try build would be cool, though I can build my own with the patch if needed. I'll get to this after I take care of some other things this morning.

Comment 6

[Daniel Veditz [:dveditz]]

Have we seen evidence of this on the 1.9.2 branch, or is it a relatively-new-build crash?

Comment 7

[Bob Clary [:bc] (inactive)]

I have seen only once instance of a related crash with signature mozilla::plugins::PluginModuleChild::ShouldContinueFromReplyTimeout. Socorro shows 300 in the last week though: https://crash-stats.mozilla.com/query/query?do_query=1&query_type=contains&query=mozilla%3A%3Aplugins%3A%3APluginModuleChild%3A%3AShouldContinueFromReplyTimeout

I have seen only one instance of a related crash with signature nsNPAPIPluginInstance::InvalidateOwner. There are no instances of this in Socorro.

I haven't been able to reproduce locally.

The original SocorroRecord where the crash automation obtained the url to test had signature mozilla::widget::WindowHook::Lookup(unsigned int) which has ~3000 crashes in the last week. https://crash-stats.mozilla.com/query/query?do_query=1&query_type=contains&query=mozilla%3A%3Awidget%3A%3AWindowHook%3A%3ALookup 

See 556524. There were two mozilla::widget::WindowHook::Lookup at sheru100 in Nov/Dec. I'll try to collect a set of urls to retest to see if I can get more information.

Johnny, without more information want to go ahead and see if this helps in other ways? Perhaps the fix will affect other "unrelated" signatures.

Comment 8

[Johnny Stenback (:jst)]

Comment on attachment 578507 [details] [diff] [review]
Possible fix.

Yeah, I think we should just land this patch, requesting review.

Comment 9

[Daniel Veditz [:dveditz]]

Looks like this applies to 1.9.2 as well

Comment 10

[Josh Aas]

Comment on attachment 578507 [details] [diff] [review]
Possible fix.

Review of attachment 578507 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/base/nsGlobalWindow.cpp
@@ +2177,5 @@
> +          // this window.
> +          mDummyJavaPluginOwner->Destroy();
> +          mDummyJavaPluginOwner = nsnull;
> +
> +          mDidInitJavaProperties = PR_FALSE;

How come we don't set mDidInitJavaProperties to false in the other places where we destroy the dummy instance, like in "nsGlobalWindow::FreeInnerObjects"?

Comment 11

[Johnny Stenback (:jst)]

All other places that tear down the dummy java plugin we're at the very end of the lifetime of the window, so it doesn't matter. But for consistency's sake it wouldn't hurt to do the same there as well.

Comment 12

[Johnny Stenback (:jst)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/025fa101a51b

Comment 13

[Johnny Stenback (:jst)]

https://hg.mozilla.org/mozilla-central/rev/025fa101a51b

Leaving this bug open though, as it's not clear whether this patch actually changes things here, so we should track this in crash-stats etc for a while before we can tell.

Comment 14

[Daniel Veditz [:dveditz]]

Probably not much chance of getting this into Fx10 at this point, but it is a safe contained fix. Are there enough crashes that we could decide in a couple of days?

Comment 15

[Daniel Veditz [:dveditz]]

If this patch did help then we should get it into the ESR at some point.

Comment 16

[Alex Keybl [:akeybl]]

Setting the ESR tracking flag to 12+ since this hasn't yet been uplifted to 11.

Comment 17

[Johnny Stenback (:jst)]

Bob, any signs of this left? Or can we even tell?

Comment 18

[Bob Clary [:bc] (inactive)]

I haven't seen any signs of this but it is hard to tell. I wonder if this is related at all to bug 724781

Comment 19

[Johnny Stenback (:jst)]

Ok, calling this fixed then. We can always reopen this bug if additional information emerges.

Comment 20

[Lukas Blakk [:lsblakk] use ?needinfo]

[Triage Comment]
Can this patch be landed as is to ESR branch?  Please nominate for approval-mozilla-esr10 if so.  See https://wiki.mozilla.org/Release_Management/ESR_Landing_Process for details.

Comment 21

[u279076]

qa- until testcase-wanted is satisfied

Comment 22

[Daniel Veditz [:dveditz]]

Comment on attachment 578507 [details] [diff] [review]
Possible fix.

This applies to ESR10

Comment 23

[Johnny Stenback (:jst)]

https://hg.mozilla.org/releases/mozilla-esr10/rev/c8a79e73f18f

Comment 24

[Al Billings [:abillings - ex-MoCo]]

Nothing much to verify here since we've never been able to reproduce the problem again.