240986 - XPCOM sample fails to start inside xpcshell

Status: RESOLVED FIXED

(Core :: XPCOM, defect)

Description

[steve.swanson]

User-Agent:       Mozilla/4.0 (compatible; MSIE 6.0; Windows NT 5.1; .NET CLR 1.1.4322)
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8a) Gecko/20040419

I've built my own XPCOM component based on the sample in the xpcom/sample 
directory.  It worked fine until I refreshed my tree to 1.7b.



Reproducible: Always
Steps to Reproduce:
1. Start xpcshell
2. var s = Components.classes["@mozilla.org/sample;1"].createInstance();


Actual Results:  
An ASSERT fires

###!!! ASSERTION: Factory creation failed: 'NS_SUCCEEDED(rv)', file 
c:/Mozilla/m
ozilla/xpcom/components/nsNativeComponentLoader.cpp, line 160
Break: at file c:/Mozilla/mozilla/xpcom/components/nsNativeComponentLoader.cpp,
line 160

and an uncaught exception propagates through the JavaScript.

Expected Results:  
This worked in Mozilla 1.7a.  It stopped working in 1.7b/1.7.  It still 
doesn't work in 1.8a.



Compiler is VS.NET 2003.

Comment 1

[steve.swanson]

Sorry, I forgot some useful information.

Debugging is tough, since the code is mostly in templates in header files.  
However, you can put a breakpoint in
  PR_FindSymbolAndLibrary()
It gets called with argument "NS_GetFrozenFunctions".  This is the failure 
which propogates back through nsGenericModule::GetClassObject().

Comment 2

[Benjamin Smedberg]

Does your component link using the standalone glue, or does it link directly
against libxpcom.so?

Comment 3

[steve.swanson]

I think it uses the standalone glue.  (I just copied xpcom/sample/Makefile.in 
and changed the names.)

Comment 4

[Benjamin Smedberg]

We were arguing about whether components should link against the standalone
glue... they really don't need to.

But if you're going to, did you call XPCOMGlueStartup and did it succeed?

Comment 5

[steve.swanson]

When I expanded the macros (analogous to those in nsSampleModule.cpp), my call 
stack looked like
  nsGenericModule::GetClassObject(...)
  nsGenericModule::Initialize(...)
  XPCOMGlueStartup(".")
  PR_FindSymbolAndLibrary("NS_GetFrozenFunctions",...)
and this is where the failure happened.

The earlier call to NS_NewGenericModule2(...) into the DLL seems to have 
succeeded.

Are you able to repro the problem?

Comment 6

[steve.swanson]

Attachment: remove XPCOM glue from Makefile.in

The answer to the question in comment #4 seems to be no.

I turned off the XPCOM glue stuff and it works for me (on Windows).

Comment 7

[Alex Fritze]

I've started seeing this problem as well now. I'm trying to get jssh [1] to work
in a firefox-like xulapp. jssh links to the xpcomglue library. When trying to
create an instance of one of the factories in the jssh module on Windows I'm
failing with the same callstack as reported by Steve: The call to
XPCOMGlueStartup fails because the "NS_GetFrozenFunctions" symbol can't be found
by nspr. I guess this is because my xulapp links dynamically against xpcom.dll,
so "NS_GetFrozenFunctions" neither appears in the executable nor any other
library nspr knows about (i.e. it hasn't been loaded by pr_LoadLibrary).

I don't quite understand why this works on Linux and with normal Mozilla,
though. Do they link statically to xpcom?


[1] http://www.croczilla.com/jssh

Comment 8

[Alex Fritze]

Attachment: patch to nsGenericFactory.cpp

There's probably better ways to fix this, but this seems easiest.

Comment 9

[Darin Fisher]

NSPR should be able to locate xpcom symbols by simply searching the default
module under windows (i.e., the result of GetModuleHandle(NULL))... i guess that
isn't working as i thought.

Comment 10

[Darin Fisher]

> Do they link statically to xpcom?

no.. however, normal mozilla loads xpcom via PR_LoadLibrary.  that definitely
means that NSPR will know where to find xpcom.dll in that case, but still ...
i'm surprised NSPR would not find xpcom symbols by searching the HINSTANCE
returned from GetModuleHandle(NULL).

Comment 11

[Darin Fisher]

I'm able to reproduce this bug on Win2k mozilla trunk.

-> me

Comment 12

[Darin Fisher]

Comment on attachment 150430 [details] [diff] [review]
patch to nsGenericFactory.cpp

I think it makes better sense to move this logic into XPCOMGlueStartup itself. 
New patch coming up.

Comment 13

[Darin Fisher]

Attachment: v2 patch

Comment 14

[Benjamin Smedberg]

Comment on attachment 151058 [details] [diff] [review]
v2 patch

This sucks, but ok.

Comment 15

[Doug Turner (:dougt)]

Comment on attachment 151058 [details] [diff] [review]
v2 patch

you can remove the	   if (!function) branch after PR_FindSymbolAndLibrary
and set xpcomFile = null in your !function test below.

bsmedberg -- what about this patch don't you like?

Comment 16

[Alexey Proskuryakov]

(In reply to comment #13)
This same issue also occurs on MacOS X (tested with Mozilla 1.7, Camino 0.8b)
The patch doesn't fix it for me, because libxpcom.dylib fails to be loaded by filename alone, 
without any path:
    if (!xpcomFile)
        libSpec.value.pathname = XPCOM_DLL;

I have tried changing libSpec.value.pathname at runtime (in gdb) to the full path to libxpcom.dylib, and 
it fixed the issue. Unfortunately, I do not know the correct way to get the full path to a dylib in Mozilla.

Comment 17

[Darin Fisher]

> you can remove the	   if (!function) branch after PR_FindSymbolAndLibrary
> and set xpcomFile = null in your !function test below.

Are you sure about that?  xpcomFile is an input argument, so i don't think i
want to only set it to null when i decide that "." doesn't work.


> bsmedberg -- what about this patch don't you like?

I think he is generally unhappy with the XPCOM glue.  This patch also sucks
because it only helps components that were built _after_ this patch gets checked
in.  That is unfortunate, and it means that components built against official
Mozilla 1.7 will have problems.

Comment 18

[Doug Turner (:dougt)]

can we do anything with nspr regarding comment #9

Comment 19

[Darin Fisher]

(In reply to comment #18)
> can we do anything with nspr regarding comment #9

I don't know.  NSPR is searching the result of GetModuleHandle(NULL), but that
is not returning symbols in the testcase.  I'm not sure why.  Any suggestions?

Comment 20

[Darin Fisher]

The documentation for GetModuleHandle says:

  "If [the] parameter is NULL, GetModuleHandle returns a handle to the file used
to create the calling process (.exe file)."

I guess that does not include searching any DLLs that are mapped into the
process at load-time.  I do not see any WIN32 APIs that would allow us to
enumerate or search all loaded libraries :-(

Comment 21

[Alexey Proskuryakov]

(In reply to comment #20)
> I do not see any WIN32 APIs that would allow us to
> enumerate or search all loaded libraries :-(

You may want look at "Enumerating All modules for a Process" sample in Windows platform SDK docs. It 
uses EnumProcessModules() for this purpose.

I do not quite get it why enumerating all loaded libraries is needed (wouldn't 
GetModuleHandle(XPCOM_DLL) suffice for Win32?), but I'm too new to Mozilla development to judge...

Comment 22

[Alexey Proskuryakov]

(In reply to comment #16)
> The patch doesn't fix it for me, because libxpcom.dylib fails to be loaded by filename alone, 
> without any path:
>     if (!xpcomFile)
>         libSpec.value.pathname = XPCOM_DLL;

As an exceptionally bad fix, the following can be done here: 
        if (!xpcomFile)
            libSpec.value.pathname = PR_GetLibraryFilePathname(XPCOM_DLL, 0);

Then, everything should work on both MacOS X and Win32, but not on other platforms that do need a 
valid address in PR_GetLibraryFilePathname, e.g. Linux. And of course, this has a memory leak, and 
works only if libxpcom.dylib is already loaded... But lets me continue the development of my 
component :)

Comment 23

[Darin Fisher]

Comment on attachment 151058 [details] [diff] [review]
v2 patch

dougt gave me approval over email for this patch.  I think we should get this
patch in and then look to improve it for OSX.

Comment 24

[Darin Fisher]

v2 patch has been checked in on the trunk.  marking FIXED.  i'll open a separate
bug for OSX.

Comment 25

[Darin Fisher]

Comment on attachment 151058 [details] [diff] [review]
v2 patch

This is a good safe patch for the 1.7 branch.

Comment 26

[Aleksey Nogin]

This commit have added an "uninitialized" warning on brad TBox:

+xpcom/glue/standalone/nsXPCOMGlue.cpp:68
+ `nsresult rv' might be used uninitialized in this function

Comment 27

[Darin Fisher]

Attachment: fix warning

from what i can tell the uninitialized variable warning is bogus, but here's a
patch that initializes |rv| to silence the warning.  i went ahead and checked
this in on the trunk.

Comment 28

[Asa Dotzler [:asa]]

Comment on attachment 151058 [details] [diff] [review]
v2 patch

a=asa for 1.7.x checkin.

Comment 29

[Darin Fisher]

fixed1.7.x

Comment 30

[Gary Johnson]

(In reply to comment #29)
> fixed1.7.x

patch to nsGenericFactory uses null;  Should this be nsnull or NULL?

fails to compile using Free MSVC  compiler