720682 - Linking jemalloc enabled libxul with external binaries fail in runtime

Status: NEW

(Core :: Memory Allocator, defect)

Description

[Jan Horak [:jhorak]]

I have a problem with linking libxul (with --enable-jemalloc) with external binary. Crash show up after calling function je_malloc_usable_size_in_advance. This symbol is undefined in libxul, however linking does not fail on unresolved symbols (which is probably fine).

[jhorak@localhost lib]$ objdump -x libxul.so |grep je_malloc_usable_size_in_advance
00000000  w      *UND*  00000000              je_malloc_usable_size_in_advance

I've checked that xulrunner binaries like xulrunner-stub, xpcshell, js, etc are linked with libmozutils.a (which contains jemalloc.o). This static library is not installed and therefore external binaries crash when trying to use libxul.
To fix my issue I had to copy libmozutils.a to my app build directory and add -Wl,--whole-archive libmozutils.a. 
We probably need to add libmozutils.a to sdk libraries.

Comment 1

[Jan Horak [:jhorak]]

Well, that doesn't solve this problem everywhere. Create dynamic library seems to be much cleaner solution. It is possible to build both static and dynamic library?

Comment 2

[Mike Hommey [:glandium]]

Just add "if (je_malloc_usable_size_in_advance)" before the calls.

Comment 3

[Jan Horak [:jhorak]]

(In reply to Mike Hommey [:glandium] from comment #2)
> Just add "if (je_malloc_usable_size_in_advance)" before the calls.
Hm, je_malloc_usable_size_in_advance is called from libxul.

Comment 4

[Mike Hommey [:glandium]]

And that's where you need to add the ifs.

Comment 5

[Tom Hindle]

Attachment: proposed patch

Patch re-allows libxul embedding on linux, which broke with firefox 10.0.

Currently one has to use a workaround like: 

echo -e "#include<stdlib.h>\nsize_t Lje_malloc_usable_size_in_advance(size_t n) { return n; }" | gcc -xc --shared - -o jemallocfix.so

LD_PRELOAD=./jemallocfix.so appthatembedslibxul

Comment 6

[lkcl]

https://bugzilla.mozilla.org/show_bug.cgi?id=728500

is this related? it shows a segfault (full stack trace with debug symbols) at start up of python-hulahop.  --disable-jemalloc "fixed" the problem.  btw, libjemalloc1 is installed but libjemalloc-dev definitely was *not* installed on the system on which xulrunner was built, yet there were absolutely no build failures: that doesn't seem right, to me.

anyway, just linking that bugreport here in case these bugs are duplicates.

Comment 7

[Shawn Wilsher :sdwilsh]

Comment on attachment 595301 [details] [diff] [review]
proposed patch

Review of attachment 595301 [details] [diff] [review]:
-----------------------------------------------------------------

r=sdwilsh

::: storage/src/mozStorageService.cpp
@@ +571,5 @@
>  
>  static int sqliteMemRoundup(int n)
>  {
> +  if (je_malloc_usable_size_in_advance)
> +  	n = je_malloc_usable_size_in_advance(n);

nit: please brace all ifs

Comment 8

[Yun Guo]

With the given patch, the crash is gone, but nothing displayed in browser area with "about:memory" url. Is it reasonable of changing code to link the jemalloc to libxul rather than firefox-bin/xulrunner-bin?

Comment 9

[Mike Hommey [:glandium]]

(In reply to Yun Guo from comment #8)
> With the given patch, the crash is gone, but nothing displayed in browser
> area with "about:memory" url.

I would expect about:memory to work, but to lack some information. I'd suggest you to look at the error console and file a separate bug.

> Is it reasonable of changing code to link the
> jemalloc to libxul rather than firefox-bin/xulrunner-bin?

It wouldn't work.

Comment 10

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

So ... can we land this?

Comment 11

[Tom Hindle]

Attachment: updated patch based upon review.

Comment 12

[Grant Gayed]

(In reply to Mike Hommey [:glandium] from comment #9)
> I would expect about:memory to work, but to lack some information.
> I'd suggest you to look at the error console and file a separate bug.

I've built XR with the patch and my case of navigating to a url like google.com no longer crashes, but it doesn't actually navigate either.  Our same code navigates fine when embedding XR10 on Windows and OS X.

Is the now-skipped call to je_malloc_usable_size_in_advance() necessary to make a case like this work, or does it not _really_ matter, in which case the problem I now see is likely unrelated (similar to what was said about about:memory)?

Comment 13

[Tom Hindle]

@Grant Gayed

I use embedded XR10 on Ubuntu via geckofx-10 (https://bitbucket.org/geckofx/geckofx-10.0/) 
I can successful navigate to sites like google.com. I am currently using the LD_PRELOAD workaround.

I guess this means the skipped je_malloc_usable_size_in_advance is unlikely to be the cause of your problem.

Comment 14

[Grant Gayed]

I've tried a variation of the LD_PRELOAD workaround and navigating now work for me as well.  I'm not sure what was wrong with the XULRunner I'd compiled.  Assuming that sqliteMemRoundup() is the only place in XULRunner that references je_malloc_usable_size_in_advance(), the attached patch seems like it should do what's needed.

Comment 15

[Yun Guo]

(In reply to Mike Hommey [:glandium] from comment #9)
About:memory displayed blank in embed mode is not caused by this patch, but it caused by the special link position for jemalloc. The nsMemoryReporterManager's init() method execute failure due to "jemalloc_stats" is not defined. 

NS_IMETHODIMP
nsMemoryReporterManager::Init()
{
  #if HAVE_JEMALLOC_STATS && defined(XP_LINUX)
    if (!jemalloc_stats)
        return NS_ERROR_FAILURE;
  #endif
  ......
}

Do you know the reason why jemalloc can't be linked to libxul.so on Linux platform, just like other platforms?