Closed Bug 612088 Opened 14 years ago Closed 14 years ago

Firefox 4.0b7 crash mainly close to startup [@ nsComponentManagerImpl::KnownModule::Description() ] with AudioService_ff4.dll, blocklist ask.com toolbar version 3.6.*

Categories

(Toolkit :: Blocklist Policy Requests, defect, P1)

x86
All
defect

Tracking

()

RESOLVED WORKSFORME
Future

People

(Reporter: scoobidiver, Assigned: morgamic)

References

Details

(Keywords: crash, qawanted, topcrash, Whiteboard: [See comment 10])

Crash Data

Attachments

(2 files, 3 obsolete files)

It is a crash signature in 4.0b7 build. It happens mainly at startup or a few minutes after startup. AudioService_ff4.dll is loaded for every crash. It is #41 top crasher in 4.0b7 for the last week. Signature nsComponentManagerImpl::KnownModule::Description() UUID f9e833d4-37ba-4a3c-ac48-cfac12101113 Time 2010-11-13 06:49:40.795874 Uptime 12835 Install Age 179534 seconds (2.1 days) since version was first installed. Product Firefox Version 4.0b7 Build ID 20101104142426 Branch 2.0 OS Windows NT OS Version 5.1.2600 Service Pack 2 CPU x86 CPU Info GenuineIntel family 15 model 2 stepping 9 Crash Reason EXCEPTION_ACCESS_VIOLATION_READ Crash Address 0x1 App Notes AdapterVendorID: 8086, AdapterDeviceID: 2562 Frame Module Signature [Expand] Source 0 xul.dll nsComponentManagerImpl::KnownModule::Description xpcom/components/nsComponentManager.cpp:977 1 xul.dll nsComponentManagerImpl::RegisterCIDEntry 2 AudioService_ff4.dll AudioService_ff4.dll@0xd94e3 3 AudioService_ff4.dll AudioService_ff4.dll@0xd9513 4 xul.dll nsComponentManagerImpl::RegisterModule xpcom/components/nsComponentManager.cpp:453 5 xul.dll nsComponentManagerImpl::ManifestBinaryComponent xpcom/components/nsComponentManager.cpp:741 6 xul.dll ParseManifestCommon xpcom/components/ManifestParser.cpp:633 7 xul.dll ParseManifest xpcom/components/ManifestParser.cpp:650 8 xul.dll nsComponentManagerImpl::RegisterManifestFile xpcom/components/nsComponentManager.cpp:654 9 xul.dll XRE_AddManifestLocation xpcom/components/nsComponentManager.cpp:2017 10 xul.dll NS_InvokeByIndex_P xpcom/reflect/xptcall/src/md/win32/xptcinvoke.cpp:102 More reports at: http://crash-stats.mozilla.com/report/list?range_value=4&range_unit=weeks&signature=nsComponentManagerImpl%3A%3AKnownModule%3A%3ADescription%28%29&version=Firefox%3A4.0b7
Version: unspecified → Trunk
this is just stupid code. 973 nsComponentManagerImpl::KnownModule::Description() const 974 { 975 nsCString s; 976 if (!mPath.IsEmpty()) { we are here: 977 mFile->GetNativePath(s); mFile is null 978 s.Insert(NS_LITERAL_CSTRING("jar:"), 0); 979 s.AppendLiteral("!/"); 980 s.Append(mPath); 981 } here we check for mFile being null 982 else if (mFile) 983 mFile->GetNativePath(s);
Blocks: packedxpi
blocking2.0: --- → ?
Assignee: nobody → mwu
What's AudioService_ff4.dll from? It appears that it's using KnownModule incorrectly. If the constructor KnownModule(nsILocalFile* aFile, const nsACString& aPath) is used, both arguments must be non-null. I'll put up a patch to enforce this.
AudioService appears to be from the ask.com toolbar, but when I installed it, I didn't get the AudioService_ff4.dll or any dll as far as I can tell.
Assignee: mwu → nobody
(In reply to comment #2) > What's AudioService_ff4.dll from? It appears that it's using KnownModule > incorrectly. If the constructor KnownModule(nsILocalFile* aFile, const > nsACString& aPath) is used, both arguments must be non-null. Which doesn't mean that we should stop doing checks before using data from external modules.
Keywords: topcrash
Attached patch restoring misplaced null check (deleted) — Splinter Review
this is a trivial null check which should just be landed.
Assignee: nobody → timeless
Status: NEW → ASSIGNED
Attachment #490911 - Flags: review?(mwu)
Attachment #490911 - Flags: approval2.0?
Attachment #490911 - Flags: review?(mwu) → review?(benjamin)
Comment on attachment 490911 [details] [diff] [review] restoring misplaced null check I don't believe this patch makes any sense. *If* we have a path, we must also have a file. I don't want to take this patch as a bandaid.
Attachment #490911 - Flags: review?(benjamin)
Attachment #490911 - Flags: review-
Attachment #490911 - Flags: approval2.0?
the logic flows through this entrypoint: 437 nsComponentManagerImpl::RegisterModule(const mozilla::Module* aModule, 438 nsILocalFile* aFile) 439 { 440 nsAutoMonitor mon(mMon); 441 442 KnownModule* m = new KnownModule(aModule, aFile); here the lack of aFile is considered "OK" 443 if (aFile) { 444 nsCOMPtr<nsIHashable> h = do_QueryInterface(aFile); 445 mKnownFileModules.Put(h, m); 446 } 447 else we land in this branch which is perfectly happy (although terribly wrong for anything which isn't our exe, oh wel): 448 mKnownStaticModules.AppendElement(m); 449 450 if (aModule->mCIDs) { 451 const mozilla::Module::CIDEntry* entry; 452 for (entry = aModule->mCIDs; entry->cid; ++entry) and we crash here: 453 RegisterCIDEntry(entry, m); AFAIK, this same entrypoint is used by actual static modules in firefox.exe, so i don't think we're in a position to just add a null check for aFile. As such, the null check in KnownModules is correct.
Attachment #490911 - Flags: review?(mwu)
In that case, mPath is empty (because this isn't a JAR module), so the code is still correct. There are three cases: Static module: mFile null, mPath empty File: mFile non-null, mPath empty JAR: mFile non-null, mPath not empty In none of these cases is it acceptable for mPath to be not-empty and mFile to be null. I would love to actually get my hands on this DLL/extension.
This crash correlates with various versions of the ask.com toolbar 3.6.*. I'm going to morph this into a blocklist request: * For Firefox 4.0b2 forward * Please block "toolbar@ask.com" version 3.6.0 to 3.6.*
Assignee: timeless → nobody
blocking2.0: ? → beta8+
Component: XPCOM → Blocklisting
Product: Core → addons.mozilla.org
QA Contact: xpcom → blocklisting
Summary: Firefox 4.0b7 crash mainly close to startup [@ nsComponentManagerImpl::KnownModule::Description() ] with AudioService_ff4.dll → Firefox 4.0b7 crash mainly close to startup [@ nsComponentManagerImpl::KnownModule::Description() ] with AudioService_ff4.dll, blocklist ask.com toolbar version 3.6.*
Version: Trunk → unspecified
-> morgamic See also bug 592258.
Assignee: nobody → morgamic
Priority: -- → P1
Whiteboard: [See comment 10]
Target Milestone: --- → 5.12.4
Kicking out of AMO milestone. Blocklist requests are not something we track with milestones / dev team.
OS: Windows XP → All
Target Milestone: 5.12.4 → Future
Depends on: 592258
Attachment #490911 - Flags: review?(mwu)
Clearing review request based on comment 10 and comment 6
Not sure this is the proper way to check for FF betas. Also note there is no Ask toolbar in http://www.mozilla.com/en-US/blocklist/, so the blitem likely needs to be inserted.
Attachment #494203 - Flags: review?(morgamic)
Attachment #494203 - Attachment is obsolete: true
Attachment #494211 - Flags: review?(morgamic)
Attachment #494203 - Flags: review?(morgamic)
Attachment #494211 - Attachment mime type: application/octet-stream → text/plain
Attachment #494211 - Attachment is obsolete: true
Attachment #494226 - Flags: review?(morgamic)
Attachment #494211 - Flags: review?(morgamic)
I worked on the DLL. I want to track this bug closely, and I'm going to talk with my management and try to convince them to let me fix this.
I was just able to reproduce this, once. I saw either f or f->mModule have a value of 1, in nsComponentManagerImpl::RegisterCIDEntry.
Attached file possibly related bug analysis (deleted) —
In trying to reproduce this, I tried to register the allegedly broken DLL 10 times in a row, in XPCShell: var file = Components.classes["@mozilla.org/file/local;1"].createInstance(Components.interfaces.nsILocalFile); file.initWithPath("binary.manifest"); for (var i = 0; i < 10; i++) Components.manager.QueryInterface(Components.interfaces.nsIComponentRegistrar).autoRegister(file); This time, it consistently crashed, 100%, by the fourth pass. What if that's what's happening? Our toolbar code may be trying to register the DLL more than once through a minor bug of our own, but each time we try, the module data degrades.
Keywords: qawanted
Juan, can we try to get repro steps for this? Apparently the dll is downloaded on the fly, potentially when you turn on certain functionality. Alex, if you could give us directions on how to test to make sure we are using the dll that would be great.
The crashes will likely be fixed by bug 616056. Ask is updating their toolbar in the future as well, which should prevent the underlying issue. For beta8 I think we should take 616056 and not blocklist these extension versions. If we still see this problem, we can talk about adding just the dll to the dll blocklist, which will also fix the crashes and allow the Ask toolbar to still work. I'm going to push this to blocking beta 9. Once beta 8 is out with bug 616056 we need to look at crash stats. If this topcrash has gone away, this bug can be duped to bug 616056. Otherwise, ww'll use this for blocklisting the dll.
blocking2.0: beta8+ → beta9+
Depends on: 616056
Attachment #494226 - Attachment is obsolete: true
Attachment #494226 - Flags: review?(morgamic)
There isn't a lot to go on to reproduce the problem. Comments in the crash reports are of no help. I tried installing the toolbar and trying different interactions with it, quitting, starting up, no crash on the latest nightly. Alex, if you have any suggestions on how to reproduce this problem let me know.
People who have the broken version of the ask toolbar are going to be stuck on beta7, because this is a startup crash. We should at least blacklist just for beta5->beta7pre just so that these people can update to beta8.
juanb: No one at our office has been able to reproduce this crash. Also, the toolbar in question is from one of our partners. I'm going to e-mail you off-list with the latest info I've got, but unfortunately it doesn't include the specific partner toolbar.
(In reply to comment #23) > People who have the broken version of the ask toolbar are going to be stuck on > beta7, because this is a startup crash. We should at least blacklist just for > beta5->beta7pre just so that these people can update to beta8. Will they even get the updated blocklist if it is a startup crash?
Doesn't look like this crash has happened in 4.0beta8 and later. Looks like bug 616056 likely fixed the underlying issue.
This bug is still listed as a FF4 beta 9 blocker. Are we still seeing no evidence of crashes with the Ask toolbar? If so, I'd like to request this bug resolved with no action taken (i.e. please don't blocklist us, it wasn't our fault :) )
Status: ASSIGNED → RESOLVED
blocking2.0: beta9+ → ---
Closed: 14 years ago
Resolution: --- → WORKSFORME
Crash Signature: [@ nsComponentManagerImpl::KnownModule::Description() ]
Product: addons.mozilla.org → Toolkit
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: