108573 - Regxpcom access violation when registering native dll

Status: VERIFIED FIXED

(Core :: XPCOM, defect)

Description

[John Nason]

From Bugzilla Helper:
User-Agent: Mozilla/4.0 (compatible; MSIE 5.01; Windows NT 5.0)
BuildID:    .9.4.1

As of Moz9.2.1/NS6.1 I was registering my native dll with no problems. I have 
since built .9.4.1 in debug and release and regxpcom keeps crashing with an 
access violation. I have been stepping through the regxpcom code(although this 
is obviously not my area of expertise) and here is what I've found:
Soon after initialization regxpcom goes through the registry scanning types and 
building a list of loaders (mLoaderData). When it comes to a 
type "text/javascript" it can not find a corresponding loader type, so it adds 
one to the list and sets the member "loader" to null.
This is where my stuff comes in:
My dll is native windows, so AutoRegisterComponent is called, and from that the 
overloaded AutoRegisterComponent in nsNativeComponentManager is called. The 
native version has a third parameter &didRegister, which seems pretty self 
explanatory.  nsComponentManagerImpl::AutoRegisterComponent loops through the 
loaders while didRegister==false. The problem is, my dll registers fine, but 
didRegister is never set. I grepped the code and there is no way for it to get 
set true, registered or otherwise. It really gets ugly because the loop does 
not break, and the next loader is tried. If you remember from earlier, when the 
registry was scanned, a loader for javascript was added, but set to null(this 
is very dubious). This new loader is then tried for the dll(=null) and memory 
chaos ensues. When stepping through the code, if I set &didRegister to true by 
force after the sucessful registration of my dll, it works fine.




Reproducible: Always
Steps to Reproduce:
1.Register a native windows dll with regxpcom
2.
3.

Actual Results:  Access violation. Method called on a null pointer.

Expected Results:  Exited after the successful registration, or added a valid 
loader for javascript.

Comment 1

[Doug Turner (:dougt)]

reporter, could you create and attach a test case?

Comment 2

[John Nason]

Attachment: Zipped up example source/dll. NB: .dsp has paths to inlcude/link that you'll need to change.

Comment 3

[Doug Turner (:dougt)]

The trunk works fine.  Need to try branch.

Comment 4

[Doug Turner (:dougt)]

*** Bug 108007 has been marked as a duplicate of this bug. ***

Comment 5

[Xiaobin Lu]

Add myself and Steve, Stanley to the list.

Comment 6

[Doug Turner (:dougt)]

Attachment: protects against null mLoaderData[i].loader's

Comment 7

[Doug Turner (:dougt)]

This fixes the problem on the trunk.  It should be backported to 0.9.4.

As for clients that have already built on 0.9.4, they must either ship a patched
version of xpcom (~450K) or hackout the correct registration functionality.  

Comment 8

[Doug Turner (:dougt)]

Comment on attachment 56965 [details] [diff] [review]
protects against null mLoaderData[i].loader's

There is a better way to do this.  instead of just protection, we want to do
creation.  new patch soon.

Comment 9

[Suresh Duddi (gone)]

Doug as we talked last night, it might be better to convert each of these
callsites to GetLoaderForType() and use the loader thus got.

Comment 10

[Doug Turner (:dougt)]

right.  new patch coming soon.

Comment 11

[Peter Janes]

Was attachment 56965 [details] [diff] [review] checked in to the trunk?  My install of nightly trunk Linux
build 2001110808 still segfaults registering the Java 1.4.0beta3 plugin.

Comment 12

[Doug Turner (:dougt)]

Attachment: Protects against null and creates loaders were needed

dp, can you review?

Comment 13

[Doug Turner (:dougt)]

Comment on attachment 57102 [details] [diff] [review]
Protects against null and creates loaders were needed

crap.  one more time.

Comment 14

[Doug Turner (:dougt)]

Comment on attachment 57102 [details] [diff] [review]
Protects against null and creates loaders were needed

actually this patch is okay.  dp, can I get a review?

Comment 15

[Xiaobin Lu]

I tested the patch and find out mostly it avoid the crash. However, I noticed 
that when use regxpcom to do unregister, it still crashes with notifying the 
listener. I attached the crash stack trace here for analysis.

nsServiceEntry::NotifyListeners() line 437 + 3 bytes
nsServiceEntry::~nsServiceEntry() line 404
nsServiceEntry::`scalar deleting destructor'(unsigned int 1) + 15 bytes
FreeServiceContractIDEntryEnumerate(PLDHashTable * 0x009054c0, PLDHashEntryHdr 
* 0x00b0132c, unsigned int 347, void * 0x00000000) line 1724 + 31 bytes
PL_DHashTableEnumerate(PLDHashTable * 0x009054c0, int (PLDHashTable *, 
PLDHashEntryHdr *, unsigned int, void *)* 0x1006dc60 
FreeServiceContractIDEntryEnumerate(PLDHashTable *, PLDHashEntryHdr *, unsigned 
int, void *), void * 0x00000000) line 601 + 34 bytes
nsComponentManagerImpl::FreeServices() line 1742 + 19 bytes
NS_ShutdownXPCOM(nsIServiceManager * 0x00000000) line 479
main(int 3, char * * 0x00901800) line 199 + 8 bytes
mainCRTStartup() line 338 + 17 bytes
KERNEL32! 77f1ba06()

And regxpcom is not always successful to register the plugins. So I do feel 
that using regxpcom is painful and error-prone.

Comment 16

[Doug Turner (:dougt)]

Xiaobin, we need to make regxpcom better.  thank you for your input.  I will
take a look at this today.

Comment 17

[Doug Turner (:dougt)]

Xiaobin, could you comment the step needed to reproduce this crash?  

Comment 18

[Suresh Duddi (gone)]

Comment on attachment 57102 [details] [diff] [review]
Protects against null and creates loaders were needed

>     for (int i = NS_COMPONENT_TYPE_NATIVE + 1; i < mNLoaderData; i++) {
>+        if (!mLoaderData[i].loader) {
>+            rv = GetLoaderForType(i, &mLoaderData[i].loader);
>+            if (NS_FAILED(rv))
>+                break;
>+        }
>         rv = mLoaderData[i].loader->AutoRegisterComponents(when, spec);

Why not make this:

     for (int i = NS_COMPONENT_TYPE_NATIVE + 1; i < mNLoaderData; i++) {
	nsCOMPtr<nsIComponentLoader> loader;
	GetLoaderForType(i, getter_AddRefs(loader));
	rv = loader->AutoRegisterComponents(when, spec);

Same for all the other changes.

Comment 19

[Doug Turner (:dougt)]

we can not assume that the loaders can always be created.  assume that the
component was deleted and not unregistered.

Comment 20

[John Nason]

Is there any effort to patch the .9.4.1 code? This is particularly important to 
those of us who need to support Netscape6.2. This is less likely but any chance 
of the patched exe getting released publicly, for the masses?

Comment 21

[Doug Turner (:dougt)]

The patch is against XPCOM.dll, not the regxpcom dll.  I will attach a patch for
regxpcom which bypasses the crash.  However, this is a hack and I do not want it
to be checked into mozilla.

Comment 22

[Doug Turner (:dougt)]

Attachment: Workaround for regxpcom - not to be checked into tree.

Comment 23

[Jaime Rodriguez, Jr.]

Adding PDT for tracking and 6.2 branch review purposes.

Comment 24

[Doug Turner (:dougt)]

Xiaobin et al.  The patch 57102 applies to the 6.0.2 branch as well as the trunk.

Comment 25

[rpotts (gone)]

Comment on attachment 57102 [details] [diff] [review]
Protects against null and creates loaders were needed

i think that if the failure cases, rather than 'break'ing out of the loop, we
should just 'continue' to allow subsequent component loaders to do their
thing...

sr=[e-mail] once this is addressed :-)

-- rick

Comment 26

[Doug Turner (:dougt)]

Attachment: with rpott's suggestions

Comment 27

[Suresh Duddi (gone)]

r=dp (ccing myself too)

Comment 28

[Doug Turner (:dougt)]

Comment on attachment 58154 [details] [diff] [review]
with rpott's suggestions

from rpotts.

Comment 29

[Xiaobin Lu]

Doug:
    Tested in Windows with Netscape 6.2 branch, the registration works fine. No 
crash and hang. However, the run "regxpcom -u $(path to plugins)" hangs, it 
seems that it never stop. Run "regxpcom -u" again, it seems ok. It can not be 
always reproduced. However, unregistration does happen if I kill the program 
myself.

Xiaobin 

Comment 30

[Doug Turner (:dougt)]

any idea why or where it is getting hung during unregistration?  I could not
reproduce this on my system.  

Comment 31

[Xiaobin Lu]

Give me one more day. I will investigate it during weekends.

Comment 32

[Joe Chou]

I tried it on Solaris with Netscape 6.2, it seemed working fine, no hanging.
Then I made a symbolic link to the registerred Java plugin, I was about to do
some Java things (going to java.sun.com, ran some liveconnect test cases, etc.).
Here are the print-out when I did rexpcom and rexpcom -u, see if they were OK:

$ ./regxpcom /export/joechou/java/jre1.4_b84/j2re1.4.0/plugin/sparc/ns610/lib>
Warning: MOZILLA_FIVE_HOME not set.
Warning: MOZILLA_FIVE_HOME not set.
Type Manifest File: /export/joechou/mozBase/moz_ns62_1113/mozilla-sparc-nightly/
mozilla/dist/bin/components/xpti.dat
nsPluginHostImpl ctor
Registration successful for /export/joechou/java/jre1.4_b84/j2re1.4.0/plugin/spa
rc/ns610/libjavaplugin_oji140.so
nsPluginHostImpl::Observe "xpcom-shutdown"
nsPluginHostImpl dtor
$

$ ./regxpcom -u /export/joechou/java/jre1.4_b84/j2re1.4.0/plugin/sparc/ns610/>
Warning: MOZILLA_FIVE_HOME not set.
Warning: MOZILLA_FIVE_HOME not set.
Type Manifest File:
/export/joechou/mozBase/moz_ns62_1113/mozilla-sparc-nightly/mozilla/dist/bin/components/xpti.dat
nsPluginHostImpl ctor
Unregistration successful for
/export/joechou/java/jre1.4_b84/j2re1.4.0/plugin/sparc/ns610/libjavaplugin_oji140.so
nsPluginHostImpl::Observe "xpcom-shutdown"
nsPluginHostImpl dtor
$

Comment 33

[Suresh Duddi (gone)]

Comment on attachment 58154 [details] [diff] [review]
with rpott's suggestions

r=dp

Comment 34

[Xiaobin Lu]

Doug:
    Attachment 58154 [details] [diff] is for Netscape 6.2 branch. I tested with this patch for 
regxpcom and regxpcom -u, both work fine. The hange I saw is due to remove 
component.reg before I did "regxpcom -u". So you can safely ignore it. So I 
think at this time, you can safely check that patch into NETSCAPE_6_2_RELEASE 
branch.
   For the trunk, I applied the attchment 57102. I still saw the crash. The 
stack trace looks like I posted on 11-09. It seems that there is some problems 
of NotifyListener() method. Also when you run regxpcom continously for 3-4 time, 
the program never stops. So at this time, I don't think if is safe to check this 
patch into trunk.

Comment 35

[Doug Turner (:dougt)]

I checked the patch into the mozilla trunk:

Checking in nsComponentManager.cpp;
/cvsroot/mozilla/xpcom/components/nsComponentManager.cpp,v  <-- 
nsComponentManager.cpp
new revision: 1.177; previous revision: 1.176

NETSCAPE_6_2_1_BRANCH coming soon....

Comment 36

[Jaime Rodriguez, Jr.]

Added ETA of 11.19 to Status Whiteboard.

Comment 37

[Doug Turner (:dougt)]

checked into netscape branch.

Checking in nsComponentManager.cpp;
/cvsroot/mozilla/xpcom/components/nsComponentManager.cpp,v  <-- 
nsComponentManager.cpp
new revision: 1.157.10.1; previous revision: 1.157
done

Leaving bug open until I hear back for 0.9.4 approval.

Comment 38

[Judson Valeski]

please drop into 0.9.4 branch and put "fixed0.9.4" in the keyword field after
landing; thanks!

Comment 39

[Doug Turner (:dougt)]

Checking in nsComponentManager.cpp;
/cvsroot/mozilla/xpcom/components/nsComponentManager.cpp,v  <-- 
nsComponentManager.cpp
new revision: 1.157.2.1; previous revision: 1.157
done

QA - Please verify this on the trunk, the 0.9.4 branch, and the 6.2.1 branch.

You can verify by running regxpcom against a component library like this:

1. move xpinstal.dll from the components directory to c:\temp\
2. delete the component.reg file
3. run regxpcom
4. launch mozilla
5. exit mozilla
6. run 'regxpcom c:\temp\xpinstal.dll'
7. note that the output state that the registration was successful.

Comment 40

[Jaime Rodriguez, Jr.]

Waiting for verification on 6.2 branch.

Comment 41

[lchiang]

jimmy lee - pls help depstein, if needed, to verify this one.  Thanks.

Comment 42

[David Epstein]

Using Doug's steps above, registration was successful for 11/20 mozilla nightly
build. Will try on 0.9.4 branch, and verify on 6.2.1 branch when it's available.

Comment 43

[Xiaobin Lu]

Following Doug's testing steps, this is my testing result.

1. Run regxpcom to register c:\temp\xpinstal.dll, runs ok
2. Repeat 1 couple times, regxpcom will never stop.
3. Run regxpcom -u c:\temp\xpinstal.dll will crash.

Since this bug has been closed, I am going to open another bugzilla bug to 
resolve this.

Comment 44

[Doug Turner (:dougt)]

*** Bug 111351 has been marked as a duplicate of this bug. ***

Comment 45

[Doug Turner (:dougt)]

based on Xiaobin's comments, reopening.  

Xiaobin, what tree are you testing this on?

Comment 46

[Xiaobin Lu]

Doug, I used Mozilla trunk to test this.

Comment 47

[David Epstein]

Using the 2001-11-25-22-0.9.4ec build, got an application error in regxpcom
(unhandled exception crash) after "regxpcom c:\temp\xpinstal.dll". No stack
trace other than:
XPCOM! 60f59250()
XPCOM! 60f5c622()
REGXPCOM! 00401051()
REGXPCOM! 0040128a()
REGXPCOM! 004012f6()
KERNEL32! 77f1ba06()

Comment 48

[Doug Turner (:dougt)]

hmm.  I could not reproduce this with this build:

http://ftp.mozilla.org/pub/mozilla/nightly/latest-0.9.4ec/mozilla-win32.zip

Could you provide a link to the build you donwnloaded?

Comment 49

[Doug Turner (:dougt)]

registration is not longer a problem... I do see a crash when unregistering
during shutdown of regxpcom.  112201.

Comment 50

[David Epstein]

verified in both 0.9.4ec and 6.2.1 branches. Registration and unregistration
works with this syntax:
> regxpcom c:\\temp\\xpinstal.dll
Registration successful for c:\temp\xpinstal.dll
> regxpcom -u c:\\temp\\xpinstal.dll
Unregistration successful for c:\temp\xpinstal.dll

Comment 51

[Xiaobin Lu]

Doug:
   Just FYI and out of my curiosity, when I repeatly ran regxpcom command for 
couple times, the program never stops.

   Also unregistration does really have problem. I still saw the crash. 

Comment 52

[Xiaobin Lu]

Tested with both Mozilla nightly optimized and debug build with xpinstal.dll. 
For optimized build:
Both regxpcom and regxpcom -u  hang

For debug build of Nov 18:
repeatly running regxpcom will make the program never stops.
regxpcom -u crashes.

Reopen the bug!!

Comment 53

[Doug Turner (:dougt)]

try breaking into the debug and see what is going on.  I can not reproduce this.

Comment 54

[Xiaobin Lu]

Doug:
   Ok, these are the steps I followed to reproduce the crash:
1. Move xpinstal.dll from component directory to c:\temp.
2. Remove the component.reg file.
3. Run regxpcom, and after that run Mozilla.
4. Exit Mozilla.
5. Run regxpcom c:\temp\xpinstal.dll.
6. That runs successfully, if you keep typing this command to see if regxpcom 
is robust or not, you will find that regxpcom will never stop. But for now, we 
can ignore that.
7. Run Mozilla and exit it.
8. Run "regxpcom -u c:\temp\xpinstal.dll", the program will crash and the stack 
trace looks like below.


nsCOMPtr_base::assign_assuming_AddRef(nsISupports * 0x00000000) line 435 + 3 
bytes
nsCOMPtr_base::assign_with_AddRef(nsISupports * 0x00000000) line 74
nsCOMPtr<nsISupports>::operator=(nsISupports * 0x00000000) line 821
FreeServiceContractIDEntryEnumerate(PLDHashTable * 0x00905770, PLDHashEntryHdr 
* 0x00b00a74, unsigned int 189, void * 0x00000000) line 1663
PL_DHashTableEnumerate(PLDHashTable * 0x00905770, int (PLDHashTable *, 
PLDHashEntryHdr *, unsigned int, void *)* 0x1006da00 
FreeServiceContractIDEntryEnumerate(PLDHashTable *, PLDHashEntryHdr *, unsigned 
int, void *), void * 0x00000000) line 601 + 34 bytes
nsComponentManagerImpl::FreeServices() line 1677 + 19 bytes
NS_ShutdownXPCOM(nsIServiceManager * 0x00000000) line 499
main(int 3, char * * 0x00901b20) line 199 + 8 bytes
mainCRTStartup() line 338 + 17 bytes
KERNEL32! 77f1ba06()

Let me know if I can provide further information. However, we really need this 
bug to be fixed.

Comment 55

[Doug Turner (:dougt)]

i know about the unregister bug.  what about registration?  are you seeing a
problem when you try to register something a absolute component?

Comment 56

[Xiaobin Lu]

Yeah, when I repeated run "regxpcom c:\temp\xpinstal.dll", the program never 
stops. 

Comment 57

[Doug Turner (:dougt)]

I can not reproduce this.  Xiaobin, do you mind bringing your notebook to the
next meeting you have a netscape.  I would be happy to debug this problem.  let
me know when the next time you will be on campus.

Comment 58

[Michael Dunn]

Since it is reopened. I am removing edt094+ and Fixed 094. Please reconfirm fix
as appropriate.

Comment 59

[Doug Turner (:dougt)]

Spoke with xiaobin and we could not reproduce a crash or hang while registering
a component.  We did however reproduce a crash while unregistering.  I opened
112201 to track this problem.

Comment 60

[Peter Janes]

Is the fix for this bug potentially related to bug 116412, where regxpcom breaks
on Linux head nightlies?

Comment 61

[Michael Dunn]

Morphed bug tracked via another bug. KW restored.

Comment 62

[David Epstein]

Tested with both the latest 0.9.4ec branch:
ftp://sweetlou/products/client/seamonkey/windows/32bit/x86/2002-01-15-22-0.9.4ec/
And latest trunk build:
ftp://sweetlou/products/client/seamonkey/windows/32bit/x86/2002-01-16-09-trunk/

regxpcom registers and unregisters successfully. used xpinstal.dll for test case.

There are a couple of bugs occurring after unregistering with the 0.9.4ec build.
After unregistering, the app doesn't relaunch (didn't see this with the trunk).
Will submit bugs for this.

Comment 63

[David Epstein]

Submitted bugzilla bug 120436 for the need for XPInstall to have an unregister
method removing category entries. Bugzilla bug 120439 addresses the relaunch
problem after unregistering (DOM/JS doesn't handle removed categories).