Last Comment Bug 706381 - Java related crash with deleted pointer in esx
: Java related crash with deleted pointer in esx
: crash
Product: Core
Classification: Components
Component: Plug-ins (show other bugs)
: 9 Branch
: x86 Windows 7
-- normal (vote)
: mozilla11
Assigned To: Johnny Stenback (:jst,
: Benjamin Smedberg [:bsmedberg]
Depends on:
Blocks: malware-attacks 532972
  Show dependency treegraph
Reported: 2011-11-29 21:06 PST by Bob Clary [:bc:]
Modified: 2015-10-16 11:41 PDT (History)
8 users (show)
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

crash report (128.87 KB, text/plain)
2011-11-29 21:06 PST, Bob Clary [:bc:]
no flags Details
log (508.77 KB, text/plain)
2011-11-29 21:06 PST, Bob Clary [:bc:]
no flags Details
Possible fix. (762 bytes, patch)
2011-12-01 23:16 PST, Johnny Stenback (:jst,
jaas: review+
akeybl: approval‑mozilla‑esr10+
Details | Diff | Splinter Review

Description User image Bob Clary [:bc:] 2011-11-29 21:06:19 PST
Created attachment 577852 [details]
crash report

0. enable Java(TM) Platform SE 6 U29
1. enable popups
2. disable malware detector
4. shutdown will be loaded which is reported as a malware attack site.

Nightly/11 Windows 7
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 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 User image Bob Clary [:bc:] 2011-11-29 21:06:59 PST
Created attachment 577853 [details]
Comment 2 User image Daniel Veditz [:dveditz] 2011-11-30 16:29:05 PST
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 User image Bob Clary [:bc:] 2011-11-30 16:49:53 PST
I'll try again. It was late and the attempts I made using wget didn't seem to catch anything useful.
Comment 4 User image Johnny Stenback (:jst, 2011-12-01 23:16:00 PST
Created attachment 578507 [details] [diff] [review]
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 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 User image Bob Clary [:bc:] 2011-12-02 07:53:43 PST
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 User image Daniel Veditz [:dveditz] 2011-12-02 14:25:15 PST
Have we seen evidence of this on the 1.9.2 branch, or is it a relatively-new-build crash?
Comment 7 User image Bob Clary [:bc:] 2011-12-04 09:14:59 PST
I have seen only once instance of a related crash with signature mozilla::plugins::PluginModuleChild::ShouldContinueFromReplyTimeout. Socorro shows 300 in the last week though:

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. 

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 User image Johnny Stenback (:jst, 2012-01-12 13:30:11 PST
Comment on attachment 578507 [details] [diff] [review]
Possible fix.

Yeah, I think we should just land this patch, requesting review.
Comment 9 User image Daniel Veditz [:dveditz] 2012-01-12 13:38:54 PST
Looks like this applies to 1.9.2 as well
Comment 10 User image Josh Aas 2012-01-12 21:59:50 PST
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 User image Johnny Stenback (:jst, 2012-01-13 12:44:01 PST
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 User image Johnny Stenback (:jst, 2012-01-18 15:50:51 PST
Comment 13 User image Johnny Stenback (:jst, 2012-01-19 09:04:54 PST

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 User image Daniel Veditz [:dveditz] 2012-01-19 13:25:03 PST
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 User image Daniel Veditz [:dveditz] 2012-02-02 13:38:12 PST
If this patch did help then we should get it into the ESR at some point.
Comment 16 User image Alex Keybl [:akeybl] 2012-02-16 15:39:41 PST
Setting the ESR tracking flag to 12+ since this hasn't yet been uplifted to 11.
Comment 17 User image Johnny Stenback (:jst, 2012-02-23 14:01:29 PST
Bob, any signs of this left? Or can we even tell?
Comment 18 User image Bob Clary [:bc:] 2012-02-23 14:26:16 PST
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 User image Johnny Stenback (:jst, 2012-03-08 13:45:00 PST
Ok, calling this fixed then. We can always reopen this bug if additional information emerges.
Comment 20 User image Lukas Blakk [:lsblakk] use ?needinfo 2012-03-20 10:33:12 PDT
[Triage Comment]
Can this patch be landed as is to ESR branch?  Please nominate for approval-mozilla-esr10 if so.  See for details.
Comment 21 User image Anthony Hughes (:ashughes) [GFX][QA][Mentor] 2012-03-29 15:12:27 PDT
qa- until testcase-wanted is satisfied
Comment 22 User image Daniel Veditz [:dveditz] 2012-04-05 12:43:06 PDT
Comment on attachment 578507 [details] [diff] [review]
Possible fix.

This applies to ESR10
Comment 23 User image Johnny Stenback (:jst, 2012-04-06 22:59:11 PDT
Comment 24 User image Al Billings [:abillings] 2012-04-09 17:09:53 PDT
Nothing much to verify here since we've never been able to reproduce the problem again.

Note You need to log in before you can comment on or make changes to this bug.