Blocklist Internet Download Manager

RESOLVED FIXED

Status

()

Toolkit
Blocklisting
RESOLVED FIXED
8 years ago
2 years ago

People

(Reporter: sicking, Unassigned)

Tracking

unspecified
x86
Windows XP
Points:
---
Bug Flags:
in-testsuite +
in-litmus +

Firefox Tracking Flags

(Not tracked)

Details

Over in bug 530968 all data is now pointing to that the strchr crash is due to the "Internet Download Manager" tool.

Currently #12 for 3.6b5, and "Internet Download Manager" is in 80% of the crashes, vs just 10% normally. So it seems responsible for the vast majority of crashes.

Damon contacted them back in late november/early december 2009. I think the only response we got was a "we'll look in to it", but so far no further action.

They install themselves by adding a registry key to
HKCU/software/mozilla/firefox/extensions
and they show up in the addons manager and can be disabled there. (They can't be uninstalled though). That means that we can use normal addon blocklisting, right? No need to resort to DLL blocklisting.

I just tried installing the latest version, and the dll that gets installed is version 5.18.6.0, which is showing up heavily in our crash stats. So the latest version is still affected unfortunately.

The install.rdf file contains the following information:

     <em:id>mozilla_cc@internetdownloadmanager.com</em:id>
     <em:name>IDM CC</em:name>
     <em:version>6.8</em:version>


So note that while their website, and the dll, is version 5.18.x, the addon version exposed to firefox is 6.8.
Of course, blocking this means we're going to get a bunch of user complains like we did last time we blocked IDM (see bug 382356).

Cc'ing Charles who was a developer who commented on the last bug.

Comment 2

8 years ago
AFAIR, we've already blocklisted IDM v2.1-3.3 from 3.0a1 and higher.  I guess this is a newer version, but just seems like blocklisting them again is a short term problem.

Comment 3

8 years ago
Hi Shawn,

Thanks for CC’ing about the issue. Nobody from Mozilla has contacted us about this problem yet.

Looked through bug 530968

We had a few reports for crash incidents that could correspond to crash dumps you were seeing. 

The crash appeared only when 2 add-ons installed on the same time: IDM CC and TabMixPlus

We were not able to reproduce the problem when IDM CC was installed without TabMixPlus.

After analyzing crash dumps we could not find that IDM CC had been really directly involved. Crash happened in Firefox native code and stack had no idmmzcc.dll entries. Although if you disabled either TabMixPlus or IDM CC the problem would disappear. Once enabled both extensions again, Firefox 3.6 b3 started to crash when opening some web pages like msn.com. The crashes related to Firefox 3.6 b3

Firefox 3.6 beta 4 seemed to have fixed all crash issues with IDM. At least when upgraded to Firefox 3.6 beta 4, our users and we could not reproduce this problem anymore.

Do you see any crashes that relate to IDM CC 5.18.6.0 and Firefox 3.6 b4 only or b5 or RC only?

Kind regards,

Charles Jones
Software engineer

Tonec Inc.
641 Lexington Avenue, 15th fl. 
New York, NY, 10022

Comment 4

8 years ago
charles: so, the interesting part of that was:

bug 530968 comment 2, by Jonas Sicking

> Dbaron helped pull out some data showing that idmmzcc.dll is indeed the caller
> of PR_SetEnv here. Turns out that our minidumps contain information about where
> in memory each module is loaded.

> Another interesting fact is that the string

> @idm/mozilla_cc.spellcheck-directory-provider

> appears higher up on the stack.

If i'm right, that could make the stack:
> strchr(null)
> PR_SetEnv(null)
> <idmmzcc.dll> | Note that it's hard to guarantee frame counts for modules where
> <idmmzcc.dll> | we don't have symbols, but essentially this is your code even
>               | though our crash reports don't properly indicate this
> nsFactoryEntry::GetFactory()
> nsComponentManagerImpl::CreateInstanceByContractID("@idm/mozilla_cc.spellcheck-directory-provider")
> nsComponentManagerImpl::GetServiceByContractID("@idm/mozilla_cc.spellcheck-directory-provider")

Comment 5

8 years ago
Just to sum up what we have here on the issue…

No matter how hard we try we cannot reproduce this startup crash. As far as I know nobody from Mozilla can reproduce this crash either.

We have analyzed crash info above. We don't call PR_SetEnv directly, but it's called from standard xpcom initialization routines that compiler imports from xpcom/gre libraries. The original code for these xpcom initialization routines has been developed by Netscape Communications Corp.

PR_SetEnv is called from GRE_AddGREToEnvironment subroutine. GRE_AddGREToEnvironment is called from XPCOMGlueStartup.

XPCOMGlueStartup can be called either from nsGenericModule::Initialize or from GRE_Startup entry point. There are no other ways PR_SetEnv could be called in idmmzcc.dll.

1 nsGenericModule::GetClassObject //defined in nsGenericFactory.cpp
2 nsGenericModule::Initialize //defined in nsGenericFactory.cpp
3 XPCOMGlueStartup // XPCOMGlueStartup("."); //defined in nsXPCOMGlue.cpp
4 GRE_AddGREToEnvironment //defined in nsXPCOMGlue.cpp, uses GRE_GetGREPath 
defined in nsGREDirServiceProvider.cpp
5 PR_SetEnv

PR_SetEnv is called to save paths in environment related to different GRE implementations which may be installed on a computer.

Maybe this old Netscape code (part of Gecko SDK) had bugs which caused crashes on xpcom startup for certain GRE/XUL configurations?

If the crash really happens in PR_SetEnv, it happens in Gecko SDK initialization code. We did not develop this code.

We can recompile the component with a newer version of Gecko SDK, but I’m not sure it will fix the problem. There is no environment to test it. Any contacts with users who experience this crash should be useful. Also crashing dumps to analyze stack in visual studio can be useful as well. If you have any non-disclosure terms, we can sign an NDA.

Kind regards,

Charles Jones, Tonec Inc.

Comment 6

8 years ago
Thanks. I think that helped.

http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/xpcom/glue/standalone/nsXPCOMGlue.cpp&rev=1.18.2.2&mark=409,423-434#409
409 static char* spEnvString = 0;
410 
411 void 
412 GRE_AddGREToEnvironment() 
413 { 
414 const char* grePath = GRE_GetGREPath(); 
415 if (!grePath) 
416 return; 
417 
418 const char* path = PR_GetEnv(XPCOM_SEARCH_KEY); 
419 if (!path) { 
420 path = ""; 
421 } 
422 
423 if (spEnvString) PR_smprintf_free(spEnvString); 
424 
425 /** 
426 * if the PATH string is longer than our static buffer, allocate a 
427 * buffer for the environment string. This buffer will be leaked at shutdown! 
428 */ 
429 bsmedberg 1.15 if (strlen(grePath) + strlen(path) + 
430 sizeof(XPCOM_SEARCH_KEY) + sizeof(XPCOM_ENV_PATH_SEPARATOR) > MAXPATHLEN*10) { 
431 if (PR_smprintf(XPCOM_SEARCH_KEY "=%s" XPCOM_ENV_PATH_SEPARATOR "%s", 
432 grePath, 
433 path)) { 
434 PR_SetEnv(spEnvString); 

I don't actually see it writing into spEnvString.

Note that it took me a really long time to even find this function. So, yes, i'd request that you just try to link against a different version of this thing (one that doesn't have that code). And see if the problem goes away.

Comment 7

8 years ago
It appears that you are linking with the "standalone glue" (xpcomglue.lib). This is correct for embedders, but *not* for components, you should be linking with xpcomglue_s.lib. Do not #define XPCOM_GLUE. See https://developer.mozilla.org/en/XPCOM_Glue

You should not be calling GRE_GetGREPath*, because as a component the GRE is already loaded and you don't need to go find it.

Comment 8

8 years ago
Our extension loaded similar to WebLock sample here:
https://developer.mozilla.org/en/Creating_XPCOM_Components/Starting_WebLock
We used the settings specified here:
https://developer.mozilla.org/en/Creating_XPCOM_Components/Setting_up_the_Gecko_SDK 
namely two preprocessor defines: XPCOM_GLUE and MOZILLA_STRICT_API.
Also we used lib files recommended on the page above including xpcomglue.lib
We used Gecko SDK 1.7 where xpcom linking/loading supposed to be frozen and should have worked for future versions of Firefox

Today we have recompiled the component with Geck SDK 1.8.0.4, removed XPCOM_GLUE and  MOZILLA_STRICT_API defines and changed xpcomglue.lib to xpcomglue_s.lib xpcom.lib as recommended above in this thread.

Now we don’t have any imports of PR_SetEnv inside our dll. Since we don’t call PR_SetEnv at all, this bug should not happen again. Also we linked plds4.lib plc4.lib embed_base_s.lib xpcomglue_s.lib xpcom.lib nspr4.lib

Is everything correct? Should we release a new version of our extension, update it in the Internet and on add-ons site?
Fantastic! Thanks Charles, timeless and bsmedberg!

(In reply to comment #8)
> Is everything correct?

I'll leave this to bsmedberg.

> Should we release a new version of our extension, update
> it in the Internet and on add-ons site?

That would be awesome! We'll be able to monitor how much crashes go down from then, which should correlate with the amount of upgrades.

Comment 10

8 years ago
It looks correct from your description, yes.

Comment 11

8 years ago
we have tested the new version well, and it seems to work just fine.

we have updated version on our site and sent it to Mozilla addons site. now it's in sandbox:

https://addons.mozilla.org/en-US/firefox/addon/6973

Note that this URL lists an obsolete version at this time.

yes, our users can update really quickly if you approve IDM CC 6.9.1 from 
sandbox and make it public promptly. it's in your interest to update the 
version on addons.mozilla.org before the final FF 3.6 release.

if you have any questions, don't hesitate to contact me
(In reply to comment #11)
> yes, our users can update really quickly if you approve IDM CC 6.9.1 from 
> sandbox and make it public promptly. it's in your interest to update the 
> version on addons.mozilla.org before the final FF 3.6 release.
Since we released yesterday, that's a little hard.  Since you have a binary add-on, you will have to send your source code to Jorge Villalobos <jorge@mozilla.com> on the AMO team to review per our policy (https://addons.mozilla.org/en-US/developers/docs/policies/reviews).

Updated

7 years ago
Flags: in-testsuite+
Flags: in-litmus+

Comment 13

6 years ago
I think we can CLOSE this bug as per comment 11. Plus, IDM has been updated many times in the last two years.
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
(Assignee)

Updated

2 years ago
Product: addons.mozilla.org → Toolkit
You need to log in before you can comment on or make changes to this bug.