Closed
Bug 736564
Opened 13 years ago
Closed 12 years ago
mozglue library should be included into xulrunner SDK
Categories
(Core :: mozglue, defect)
Core
mozglue
Tracking
()
RESOLVED
FIXED
mozilla15
People
(Reporter: romaxa, Assigned: glandium)
References
Details
Attachments
(4 files, 2 obsolete files)
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
Reporter | ||
Comment 1•13 years ago
|
||
Reporter | ||
Comment 2•13 years ago
|
||
Reporter | ||
Comment 3•13 years ago
|
||
./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
Reporter | ||
Comment 4•13 years ago
|
||
Sounds like embedding just don't see these functions and unable to call it.
Reporter | ||
Comment 5•13 years ago
|
||
Ok, seems adding:
-Wl,--whole-archive $OBJ_DIST_BIN/../lib/libmozglue.a -lpthread -Wl,--no-whole-archive -rdynamic
Helps to solve this problem
Reporter | ||
Comment 6•13 years ago
|
||
and libmozglue.a not in SDK
Reporter | ||
Comment 7•13 years ago
|
||
by the same reason it fail to load jemalloc functions
Reporter | ||
Comment 8•13 years ago
|
||
Comment 9•13 years ago
|
||
(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.
Reporter | ||
Updated•13 years ago
|
Summary: Hash tables gecko update seems broke Embedding XPCOM init → mozglue library should be included into xulrunner SDK
Comment 10•13 years ago
|
||
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•13 years ago
|
||
(or we should separate mfbt from mozutils)
Assignee | ||
Updated•13 years ago
|
Component: XPCOM → mozglue
QA Contact: xpcom → mozglue
Comment 12•13 years ago
|
||
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.
Reporter | ||
Comment 13•13 years ago
|
||
(In reply to Benjamin Smedberg [:bsmedberg] from comment #11)
> (or we should separate mfbt from mozutils)
And jemalloc?
Reporter | ||
Comment 14•13 years ago
|
||
> 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•13 years ago
|
||
(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•13 years ago
|
||
Maybe we should just undo Bug 677501, which combined them in the first place...
Assignee | ||
Comment 17•13 years ago
|
||
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•13 years ago
|
||
Why can't we just remove jemalloc from mozglue (on Linux) and ship it separately?
Assignee | ||
Comment 19•13 years ago
|
||
(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•13 years ago
|
||
Why not? In that case embedders can be required to link against mozglue and can choose whether to link against jemalloc, right?
Assignee | ||
Comment 21•13 years ago
|
||
(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•13 years ago
|
||
Why not? pyxpcom would not link against jemalloc, would link against mozglue, and it would work correctly, no?
Reporter | ||
Comment 23•13 years ago
|
||
So what should be done here?
Assignee | ||
Comment 25•12 years ago
|
||
Attachment #617812 -
Flags: review?(ted.mielczarek)
Attachment #617812 -
Flags: review?(benjamin)
Updated•12 years ago
|
Attachment #617812 -
Flags: review?(ted.mielczarek) → review+
Assignee | ||
Updated•12 years ago
|
Attachment #606913 -
Attachment is obsolete: true
Attachment #606913 -
Flags: review?(benjamin)
Updated•12 years ago
|
Attachment #617812 -
Flags: review?(benjamin) → review+
Reporter | ||
Updated•12 years ago
|
Keywords: checkin-needed
Assignee | ||
Comment 26•12 years ago
|
||
Target Milestone: --- → mozilla15
Assignee | ||
Updated•12 years ago
|
OS: Linux → All
Hardware: ARM → All
Comment 27•12 years ago
|
||
Please take checkin-needed off the bug when a patch lands on inbound.
Flags: in-testsuite-
Keywords: checkin-needed
Assignee | ||
Comment 28•12 years ago
|
||
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?
Assignee: mh+mozilla → nobody
Component: mozglue → Networking
OS: All → Linux
QA Contact: mozglue → networking
Hardware: All → ARM
Target Milestone: mozilla15 → mozilla14
Version: Trunk → Other Branch
Assignee | ||
Updated•12 years ago
|
Assignee: nobody → mh+mozilla
Component: Networking → mozglue
OS: Linux → All
QA Contact: networking → mozglue
Hardware: ARM → All
Target Milestone: mozilla14 → mozilla15
Version: Other Branch → Trunk
Comment 29•12 years ago
|
||
libmemory.a is fine.
Assignee | ||
Comment 30•12 years ago
|
||
Attachment #620713 -
Flags: review?(ted.mielczarek)
Assignee | ||
Updated•12 years ago
|
Whiteboard: [leave open after inbound merge]
Updated•12 years ago
|
Attachment #620713 -
Flags: review?(ted.mielczarek) → review+
Assignee | ||
Comment 31•12 years ago
|
||
Whiteboard: [leave open after inbound merge]
Comment 32•12 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Comment 33•12 years ago
|
||
Reporter | ||
Comment 34•12 years ago
|
||
Attachment #606850 -
Attachment is obsolete: true
Assignee | ||
Updated•12 years ago
|
Attachment #621506 -
Attachment mime type: application/x-sh → text/plain
You need to log in
before you can comment on or make changes to this bug.
Description
•