Last Comment Bug 736564 - mozglue library should be included into xulrunner SDK
: mozglue library should be included into xulrunner SDK
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: mozglue (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla15
Assigned To: Mike Hommey [:glandium]
:
: Mike Hommey [:glandium]
Mentors:
Depends on: 751871 763152
Blocks: 729940
  Show dependency treegraph
 
Reported: 2012-03-16 12:26 PDT by Oleg Romashin (:romaxa)
Modified: 2013-08-12 07:29 PDT (History)
8 users (show)
ryanvm: in‑testsuite-
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Simple embedding test (8.71 KB, text/x-c++src)
2012-03-17 03:56 PDT, Oleg Romashin (:romaxa)
no flags Details
Make file (345 bytes, application/x-sh)
2012-03-17 03:57 PDT, Oleg Romashin (:romaxa)
no flags Details
Add mozglue to SDK (1.26 KB, patch)
2012-03-17 16:23 PDT, Oleg Romashin (:romaxa)
no flags Details | Diff | Splinter Review
Ship mozglue and jemalloc libraries in the SDK (4.08 KB, patch)
2012-04-24 00:37 PDT, Mike Hommey [:glandium]
benjamin: review+
ted: review+
Details | Diff | Splinter Review
Fixup for - Ship libmemory.a in the SDK instead of libjemalloc.a (3.19 KB, patch)
2012-05-03 08:32 PDT, Mike Hommey [:glandium]
ted: review+
Details | Diff | Splinter Review
New make.sh which work with latest m-c (395 bytes, text/plain)
2012-05-06 23:03 PDT, Oleg Romashin (:romaxa)
no flags Details

Description Oleg Romashin (:romaxa) 2012-03-16 12:26:48 PDT
I'm building Mozilla Embedding (in process) and found that it stopped working recently after landing patch:
https://bug729940.bugzilla.mozilla.org/attachment.cgi?id=601861

Here is embedding I use: http://hg.mozilla.org/users/romaxa_gmail.com/qtmozembed
If I just revert whole Gecko hash patch it works fine.

here is the crash backtrace:
#0  0x00000000 in ?? ()
#1  0x476dd9b0 in PL_DHashTableOperate (table=0x1b01fb8, key=0x47eb2858, op=PL_DHASH_LOOKUP)
    at obj-build-linaro-qt-qws-dbg/xpcom/build/pldhash.cpp:612
#2  0x4770936e in GetEntry (aKey=..., this=0x1b01fb8) at ../../dist/include/nsTHashtable.h:170
#3  nsBaseHashtable<nsIDHashKey, nsFactoryEntry*, nsFactoryEntry*>::Get (this=0x1b01fb8, aKey=...) at ../../dist/include/nsBaseHashtable.h:148
#4  0x477094fa in nsComponentManagerImpl::RegisterCIDEntry (this=0x1b01f78, aEntry=0x480c0364, aModule=0x1b19b70)
    at xpcom/components/nsComponentManager.cpp:456
#5  0x4770b0be in nsComponentManagerImpl::RegisterModule (this=0x1b01f78, aModule=0x47fb37f4, aFile=<optimized out>)
    at xpcom/components/nsComponentManager.cpp:430
#6  0x4770bd02 in nsComponentManagerImpl::Init (this=0x1b01f78) at xpcom/components/nsComponentManager.cpp:380
#7  0x476e1446 in NS_InitXPCOM2_P (result=0x0, binDirectory=<optimized out>, appFileLocationProvider=<optimized out>)
    at xpcom/build/nsXPComInit.cpp:490
#8  0x46bfbe62 in XRE_InitEmbedding2 (aLibXULDirectory=0x1ad77d8, aAppDirectory=0x1ad7910, aAppDirProvider=<optimized out>)
    at toolkit/xre/nsEmbedFunctions.cpp:202
#9  0x0000dfbe in InitEmbedding (aProfilePath=<optimized out>) at gecko/EmbeddingSetup.cpp:289
#10 0x0000e986 in MozApp (aProfilePath=0x15524 "/home/romaxa/mozfoc", this=0x1aae9a8) at gecko/MozApp.cpp:104
#11 MozApp::CreateMozApp (aProfilePath=0x15524 "/home/romaxa/mozfoc") at gecko/MozApp.cpp:70
#12 0x00011200 in GeckoPreLoaded (aLoader=<optimized out>, aData=0xbec1d554) at microb.cpp:40
#13 0x00011124 in GeckoLoader::timerfired (this=0xbec1d530) at geckoloader.cpp:30
#14 0x4112ac78 in QMetaCallEvent::placeMetaCall (this=<optimized out>, object=<optimized out>) at kernel/qobject.cpp:525
#15 0x41138cac in QObject::event (this=0xbec1d530, e=<optimized out>) at kernel/qobject.cpp:1192
#16 0x404671b4 in notify_helper (e=0x1aae968, receiver=0xbec1d530, this=<optimized out>) at kernel/qapplication.cpp:4550
#17 QApplicationPrivate::notify_helper (this=<optimized out>, receiver=0xbec1d530, e=0x1aae968) at kernel/qapplication.cpp:4522
#18 0x4046e258 in QApplication::notify (this=0xbec1d540, receiver=0xbec1d530, e=0x1aae968) at kernel/qapplication.cpp:4411
#19 0x4110d1d0 in QCoreApplication::notifyInternal (this=0xbec1d540, receiver=<optimized out>, event=0x1aae968) at kernel/qcoreapplication.cpp:876
#20 0x4111248c in sendEvent (event=0x1aae968, receiver=0xbec1d530) at ../../include/QtCore/../../src/corelib/kernel/qcoreapplication.h:231
#21 QCoreApplicationPrivate::sendPostedEvents (receiver=0x0, event_type=<optimized out>, data=0x1a65a88) at kernel/qcoreapplication.cpp:1497
#22 0x4050806c in sendPostedEvents () at ../../include/QtCore/../../src/corelib/kernel/qcoreapplication.h:236
#23 QEventDispatcherQWS::processEvents (this=0x1a66488, flags=...) at kernel/qeventdispatcher_qws.cpp:92
#24 0x4110a1e4 in QEventLoop::processEvents (this=<optimized out>, flags=...) at kernel/qeventloop.cpp:149
#25 0x4110a468 in QEventLoop::exec (this=0xbec1d4fc, flags=...) at kernel/qeventloop.cpp:200
#26 0x41112858 in QCoreApplication::exec () at kernel/qcoreapplication.cpp:1148
#27 0x0000d5ec in main (argc=3, argv=<optimized out>) at microb.cpp:78
Comment 1 Oleg Romashin (:romaxa) 2012-03-17 03:56:51 PDT
Created attachment 606849 [details]
Simple embedding test
Comment 2 Oleg Romashin (:romaxa) 2012-03-17 03:57:49 PDT
Created attachment 606850 [details]
Make file
Comment 3 Oleg Romashin (:romaxa) 2012-03-17 04:01:28 PDT
./embeddingtest /path/to/firefox/folder
#4  0x00000000 in ?? ()
#5  0xb5bdef7c in HashKey (aKey=0xb6585658) at ../../dist/include/nsHashKeys.h:391
#6  nsTHashtable<nsBaseHashtableET<nsIDHashKey, nsFactoryEntry*> >::s_HashKey (table=0x9886e40, key=0xb6585658) at ../../dist/include/nsTHashtable.h:442
#7  0xb5ba7b29 in PL_DHashTableOperate (table=0x9886e40, key=0xb6585658, op=PL_DHASH_LOOKUP)
    at xpcom/build/pldhash.cpp:611
#8  0xb5bdf802 in GetEntry (aKey=..., this=0x9886e40) at ../../dist/include/nsTHashtable.h:170
#9  nsBaseHashtable<nsIDHashKey, nsFactoryEntry*, nsFactoryEntry*>::Get (this=0x9886e40, aKey=...) at ../../dist/include/nsBaseHashtable.h:148
#10 0xb5bdf9f1 in nsComponentManagerImpl::RegisterCIDEntry (this=0x9886e00, aEntry=0xb71205c8, aModule=0x989e960)
    at xpcom/components/nsComponentManager.cpp:456
#11 0xb5be2079 in nsComponentManagerImpl::RegisterModule (this=0x9886e00, aModule=0xb71424b0, aFile=0x0)

#5  0xb5bdef7c in HashKey (aKey=0xb6585658) at ../../dist/include/nsHashKeys.h:391
391	    return mozilla::HashBytes(aKey, sizeof(KeyType));
(gdb) p aKey
$1 = (nsIDHashKey::KeyTypePointer) 0xb6585658
(gdb) x/wa 0xb6585658
0xb6585658 <_ZL20kComponentManagerCID>:	0x91775d60
Comment 4 Oleg Romashin (:romaxa) 2012-03-17 04:03:51 PDT
Sounds like embedding just don't see these functions and unable to call it.
Comment 5 Oleg Romashin (:romaxa) 2012-03-17 15:56:20 PDT
Ok, seems adding:
-Wl,--whole-archive $OBJ_DIST_BIN/../lib/libmozglue.a -lpthread  -Wl,--no-whole-archive -rdynamic

Helps to solve this problem
Comment 6 Oleg Romashin (:romaxa) 2012-03-17 16:07:12 PDT
and libmozglue.a not in SDK
Comment 7 Oleg Romashin (:romaxa) 2012-03-17 16:21:59 PDT
by the same reason it fail to load jemalloc functions
Comment 8 Oleg Romashin (:romaxa) 2012-03-17 16:23:34 PDT
Created attachment 606913 [details] [diff] [review]
Add mozglue to SDK
Comment 9 Justin Lebar (not reading bugmail) 2012-03-17 17:27:56 PDT
(In reply to Oleg Romashin (:romaxa) from comment #7)
> by the same reason it fail to load jemalloc functions

Yeah, it's HashBytes, which is used to hash nsID's.
Comment 10 Benjamin Smedberg [:bsmedberg] 2012-03-19 10:50:08 PDT
This patch seems to imply that we're going to require all embedders to link their binary against mozglue/jemalloc. That has previously not been the case, and will completely break some existing usages like python embeddings where you don't modify the python binary.

If we're going to do this, I think it should be proposed more explicitly and we should understand the tradeoffs. For now we should not require this linkage, and functions such as hashbytes should be moved to a different library (such as the XPCOM glue) that is required and doesn't have to be linked to the root binary.
Comment 11 Benjamin Smedberg [:bsmedberg] 2012-03-19 10:51:05 PDT
(or we should separate mfbt from mozutils)
Comment 12 Justin Lebar (not reading bugmail) 2012-03-19 10:53:53 PDT
I understand not wanting to link against jemalloc, but what about MOZ_ASSERT, which currently lives out-of-line in mozglue, or other functions we might want to define not-inline in mfbt?

I can just make HashBytes inline, if that's satisfactory to everyone.  I don't want to move it into a Gecko-only library if I can help it.
Comment 13 Oleg Romashin (:romaxa) 2012-03-19 10:53:53 PDT
(In reply to Benjamin Smedberg  [:bsmedberg] from comment #11)
> (or we should separate mfbt from mozutils)

And jemalloc?
Comment 14 Oleg Romashin (:romaxa) 2012-03-19 11:00:45 PDT
> I can just make HashBytes inline, if that's satisfactory to everyone.  I
> don't want to move it into a Gecko-only library if I can help it.
yep, making HashBytes as inline would work too, that was a first thing I did. should I attach patch with that?
Comment 15 Justin Lebar (not reading bugmail) 2012-03-19 11:02:57 PDT
(In reply to Oleg Romashin (:romaxa) from comment #14)
> > I can just make HashBytes inline, if that's satisfactory to everyone.  I
> > don't want to move it into a Gecko-only library if I can help it.
> yep, making HashBytes as inline would work too, that was a first thing I
> did. should I attach patch with that?

That's fine with me, at least until we get the larger mess sorted out.
Comment 16 Benjamin Smedberg [:bsmedberg] 2012-03-19 11:03:51 PDT
Maybe we should just undo Bug 677501, which combined them in the first place...
Comment 17 Mike Hommey [:glandium] 2012-03-19 11:07:29 PDT
Essentially, this is a linux-only problem. Mac and windows don't have the problem because mozglue is a shared library there.

For embedding on linux, we should ship libmozglue.a as part of the SDK, but as bsmedberg says, that doesn't solve the problem for e.g. python. For that one or similar other cases, what could work is a separate libmozglue dynamic library, that would come with the SDK as well (i guess). The embedding app would dlopen it before loading libxul/libxpcom, and the dynamic linker would take the missing symbols from there.

Alternatively, we could have libmozglue.a split in a static and a dynamic part. The static part would be optional when linking (mostly, jemalloc), and the dynamic part would be loaded at runtime.

Moving mfbt to the XPCOM glue would make it impossible to use in the custom dynamic linker, which lives in libmozglue (and it currently uses MOZ_ASSERT and other stuff).
Comment 18 Benjamin Smedberg [:bsmedberg] 2012-03-19 11:15:24 PDT
Why can't we just remove jemalloc from mozglue (on Linux) and ship it separately?
Comment 19 Mike Hommey [:glandium] 2012-03-19 11:19:26 PDT
(In reply to Benjamin Smedberg  [:bsmedberg] from comment #18)
> Why can't we just remove jemalloc from mozglue (on Linux) and ship it
> separately?

that wouldn't solve the embedding problem.
Comment 20 Benjamin Smedberg [:bsmedberg] 2012-03-21 10:45:09 PDT
Why not? In that case embedders can be required to link against mozglue and can choose whether to link against jemalloc, right?
Comment 21 Mike Hommey [:glandium] 2012-03-21 14:57:06 PDT
(In reply to Benjamin Smedberg  [:bsmedberg] from comment #20)
> Why not? In that case embedders can be required to link against mozglue and
> can choose whether to link against jemalloc, right?

That doesn't solve the problem for python you mentioned in comment 10
Comment 22 Benjamin Smedberg [:bsmedberg] 2012-03-22 07:31:05 PDT
Why not? pyxpcom would not link against jemalloc, would link against mozglue, and it would work correctly, no?
Comment 23 Oleg Romashin (:romaxa) 2012-04-19 00:59:51 PDT
So what should be done here?
Comment 24 Mike Hommey [:glandium] 2012-04-19 02:44:54 PDT
Let me take this.
Comment 25 Mike Hommey [:glandium] 2012-04-24 00:37:40 PDT
Created attachment 617812 [details] [diff] [review]
Ship mozglue and jemalloc libraries in the SDK
Comment 27 Ryan VanderMeulen [:RyanVM] 2012-05-03 03:15:53 PDT
Please take checkin-needed off the bug when a patch lands on inbound.
Comment 28 Mike Hommey [:glandium] 2012-05-03 08:04:44 PDT
So, I landed this today, but I found out this creates a problem with upcoming jemalloc3. Basically, we also need to put the stuff from memory/build in the jemalloc library we ship in the SDK. Doing this the easy way (without modifying current LIBRARY_NAMEs and so on) would mean shipping a "libmemory.a" library instead of "libjemalloc.a". On one hand, libmemory is a pretty generic name, on the other hand, it has the advantage of allowing any malloc library to be used without having to rename the library shipped in the SDK, and thus avoiding third parties some headaches.

bsmedberg, would you be okay with shipping a libmemory.a in the SDK, or do you prefer libjemalloc.a, or some other name?
Comment 29 Benjamin Smedberg [:bsmedberg] 2012-05-03 08:14:57 PDT
libmemory.a is fine.
Comment 30 Mike Hommey [:glandium] 2012-05-03 08:32:33 PDT
Created attachment 620713 [details] [diff] [review]
Fixup for  - Ship libmemory.a in the SDK instead of libjemalloc.a
Comment 32 Ed Morley [:emorley] 2012-05-04 02:56:40 PDT
https://hg.mozilla.org/mozilla-central/rev/4b4f59ac01ac
Comment 34 Oleg Romashin (:romaxa) 2012-05-06 23:03:55 PDT
Created attachment 621506 [details]
New make.sh which work with latest m-c

Note You need to log in before you can comment on or make changes to this bug.