Closed
Bug 1009624
Opened 10 years ago
Closed 10 years ago
startup crash in nsXULPrototypeDocument::GetCompilationGlobal()
Categories
(Core :: XUL, defect)
Tracking
()
VERIFIED
FIXED
mozilla32
People
(Reporter: smaug, Assigned: smaug)
References
Details
(Keywords: crash, topcrash-win)
Crash Data
Attachments
(2 files)
1.02 KB,
patch
|
bholley
:
review+
|
Details | Diff | Splinter Review |
2.41 KB,
patch
|
bholley
:
review+
Sylvestre
:
approval-mozilla-aurora+
Sylvestre
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
+++ This bug was initially created as a clone of Bug #994335 +++ There are similar but not the same stacks as what Bug 994335 was filed for. This bug is about the following kinds of crashes. (Unfortunately crash-stat doesn't give sane line numbers) This bug was filed from the Socorro interface and is https://crash-stats.mozilla.com/report/index/5749d746-5061-4f94-bd32-f6d932140506 ============================================================= Frame Module Signature Source 0 xul.dll nsXULPrototypeDocument::GetCompilationGlobal() content/xul/document/src/nsXULPrototypeDocument.cpp 1 xul.dll nsXULPrototypeScript::Compile(wchar_t const *,int,nsIURI *,unsigned int,nsIDocument *,nsXULPrototypeDocument *,nsIOffThreadScriptReceiver *) content/xul/content/src/nsXULElement.cpp 2 xul.dll mozilla::dom::XULDocument::OnStreamComplete(nsIStreamLoader *,nsISupports *,tag_nsresult,unsigned int,unsigned char const *) content/xul/document/src/XULDocument.cpp 3 xul.dll nsStreamLoader::OnStopRequest(nsIRequest *,nsISupports *,tag_nsresult) netwerk/base/src/nsStreamLoader.cpp 4 xul.dll nsJARChannel::OnStopRequest(nsIRequest *,nsISupports *,tag_nsresult) modules/libjar/nsJARChannel.cpp 5 xul.dll nsInputStreamPump::OnStateStop() netwerk/base/src/nsInputStreamPump.cpp 6 xul.dll nsInputStreamPump::OnInputStreamReady(nsIAsyncInputStream *) netwerk/base/src/nsInputStreamPump.cpp 7 xul.dll nsInputStreamReadyEvent::Run() xpcom/io/nsStreamUtils.cpp 8 xul.dll nsThread::ProcessNextEvent(bool,bool *) xpcom/threads/nsThread.cpp 9 xul.dll NS_ProcessNextEvent(nsIThread *,bool) xpcom/glue/nsThreadUtils.cpp 10 xul.dll mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate *) ipc/glue/MessagePump.cpp
Comment 1•10 years ago
|
||
(In reply to Olli Pettay [:smaug] from comment #0) > (Unfortunately crash-stat doesn't give sane line numbers) Does looking into minidumps via a debugger help there?
Assignee | ||
Comment 2•10 years ago
|
||
No idea.
Assignee | ||
Comment 3•10 years ago
|
||
BTW, brian, does the crash ring any bells. This is a regression from Bug 897655, as far as I see.
Flags: needinfo?(bhackett1024)
Comment 4•10 years ago
|
||
No, it doesn't. Bug 897655 made some changes around here but why would it suddenly start crashing 6 months after landing?
Flags: needinfo?(bhackett1024)
Assignee | ||
Comment 5•10 years ago
|
||
Given that Bug 994335 had the same crash address 0x18 we could try this kind of patch. Still no idea how we could in either bugs end up to null proto.
Attachment #8421914 -
Flags: review?(bobbyholley)
Assignee | ||
Comment 6•10 years ago
|
||
(In reply to Brian Hackett (:bhackett) from comment #4) > No, it doesn't. Bug 897655 made some changes around here but why would it > suddenly start crashing 6 months after landing? We have seen this bug for ages, starting from last August.
Assignee | ||
Comment 7•10 years ago
|
||
Er, we've seen this or bug 994335 since last August.
Updated•10 years ago
|
Attachment #8421914 -
Flags: review?(bobbyholley) → review+
Assignee | ||
Comment 8•10 years ago
|
||
https://tbpl.mozilla.org/?tree=Mozilla-Inbound&rev=64a7c3255059
Assignee | ||
Comment 9•10 years ago
|
||
Ok, this is actually an ancient bug. The code just used to live in a different place http://hg.mozilla.org/releases/mozilla-release/annotate/d20d499b219f/content/xul/document/src/XULDocument.cpp#l3536 is the same issue.
Assignee | ||
Comment 10•10 years ago
|
||
Er, crash-stat link https://crash-stats.mozilla.com/report/index/e61366d6-abdd-4f3b-9e67-159172140513 to the old crash.
Assignee | ||
Updated•10 years ago
|
Keywords: leave-open
Assignee | ||
Comment 11•10 years ago
|
||
Bug 993772 has probably fixed this.
Assignee | ||
Comment 12•10 years ago
|
||
Better patch coming, for beta. This should be fixed elsewhere. https://tbpl.mozilla.org/?tree=Mozilla-Inbound&rev=19bd36676edf
Assignee | ||
Comment 13•10 years ago
|
||
(In reply to Olli Pettay [:smaug] from comment #12) > Better patch coming Or not. No idea yet how to come up with something safe and certainly non-leaky. Kairo, how badly does this show up in beta? This one should be fixed on Aurora already.
Flags: needinfo?(kairo)
Assignee | ||
Comment 14•10 years ago
|
||
Or I guess this might be safe enough. Just use the global of the master prototype. The document should always have the master, but may not have current prototype. (Trying to get this pushed to try, but it is closed)
Attachment #8422081 -
Flags: review?(bobbyholley)
Assignee | ||
Comment 15•10 years ago
|
||
https://tbpl.mozilla.org/?tree=Try&rev=e0f7efe51b2e
Comment 16•10 years ago
|
||
(In reply to Olli Pettay [:smaug] from comment #10) > Er, crash-stat link > https://crash-stats.mozilla.com/report/index/e61366d6-abdd-4f3b-9e67-159172140513 > to the old crash. Interestingly, mozilla::dom::XULDocument::OnStreamComplete was a low-level signature, low enough that we never filed a bug for it, but nsXULPrototypeDocument::GetCompilationGloba is pretty high up in the stats. (In reply to Olli Pettay [:smaug] from comment #13) > Kairo, how badly does this show up in beta? This one should be fixed on > Aurora already. It's #15 in 29.0.1 release, #11 in 30 beta, but you are right in that it does not show up on 31 Aurora or 32 Nightly any more.
Crash Signature: [@ nsXULPrototypeDocument::GetCompilationGlobal()] → [@ nsXULPrototypeDocument::GetCompilationGlobal()]
[@ mozilla::dom::XULDocument::OnStreamComplete(nsIStreamLoader*, nsISupports*, tag_nsresult, unsigned int, unsigned char const*)]
Flags: needinfo?(kairo)
Updated•10 years ago
|
status-firefox29:
--- → affected
status-firefox30:
--- → affected
status-firefox31:
--- → unaffected
tracking-firefox30:
--- → +
Comment 17•10 years ago
|
||
Comment on attachment 8422081 [details] [diff] [review] v2 Review of attachment 8422081 [details] [diff] [review]: ----------------------------------------------------------------- I guess these compilations globals are all equivalent anyway. This looks fine for beta. Can you also file a bug (and probably mark it as mentor= for removing the now-unnecessary aProtoDoc argument to ::Compile on master?
Attachment #8422081 -
Flags: review?(bobbyholley) → review+
Assignee | ||
Comment 18•10 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/b2d759db4dcc
Keywords: leave-open
Assignee | ||
Comment 19•10 years ago
|
||
Comment on attachment 8422081 [details] [diff] [review] v2 [Approval Request Comment] Bug caused by (feature/regressing bug #): ancient stuff, but something has made this happen more often recently User impact if declined: crashes Testing completed (on m-c, etc.): just landed to m-i Risk to taking this patch (and alternatives if risky): This is the safest fix I can think of, but this code is old and we don't have too many tests for it. So, a bit risky. String or IDL/UUID changes made by this patch: NA Note, this bug has been fixed on trunk and Aurora already, but I landed the patch to m-i too in order to see if NS_ENSURE_STATE(aProtoDoc); causes issues. (Passing PrototypeDocument to Compile is no-op in Aurora and trunk). We don't have any way to reproduce this atm, but based on stack traces and code inspection this should help.
Attachment #8422081 -
Flags: approval-mozilla-beta?
Attachment #8422081 -
Flags: approval-mozilla-aurora?
https://hg.mozilla.org/mozilla-central/rev/b2d759db4dcc
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla32
Comment 21•10 years ago
|
||
Olli, 31 is marked as unaffected, is there a reason why you request the uplift for aurora then? I am going to approve only beta for now.
Flags: needinfo?(bugs)
Updated•10 years ago
|
Attachment #8422081 -
Flags: approval-mozilla-beta? → approval-mozilla-beta+
Assignee | ||
Comment 22•10 years ago
|
||
Because I'd like to see if NS_ENSURE_STATE(aProtoDoc); causes any problems.
Flags: needinfo?(bugs)
Updated•10 years ago
|
Attachment #8422081 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Comment 23•10 years ago
|
||
https://hg.mozilla.org/releases/mozilla-aurora/rev/f961ef4e1c14 https://hg.mozilla.org/releases/mozilla-beta/rev/28cb773f6e23
Updated•10 years ago
|
status-b2g-v1.4:
--- → fixed
Comment 25•10 years ago
|
||
(In reply to Olli Pettay [:smaug] from comment #19) > We don't have any way to reproduce this atm, but based on stack traces and > code inspection this should help. marking as [qa-]
Keywords: verifyme
Whiteboard: [qa-]
Comment 26•10 years ago
|
||
We can observe crash stats to verify this is fixed. It is on my radar for the next beta(6) as this crash is currently #11 on Fx30.
QA Whiteboard: [qa+]
Whiteboard: [qa-]
Comment 27•10 years ago
|
||
crash stats show that this has been fixed across all channels, including no crashes with Fx30b6.
Status: RESOLVED → VERIFIED
Updated•10 years ago
|
QA Whiteboard: [qa+] → [qa!]
You need to log in
before you can comment on or make changes to this bug.
Description
•