Jan Horak [:jhorak] Reporter
	Description • 13 years ago

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.

	Jan Horak [:jhorak] Reporter
	Comment 1 • 13 years ago

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?

	Mike Hommey [:glandium]
	Comment 2 • 13 years ago

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

	Jan Horak [:jhorak] Reporter
	Comment 3 • 13 years ago

(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.

	Mike Hommey [:glandium]
	Comment 4 • 13 years ago

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

	Tom Hindle
	Comment 5 • 13 years ago

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

	lkcl
	Comment 6 • 13 years ago

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.

	Shawn Wilsher :sdwilsh
	Comment 7 • 13 years ago

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

	Yun Guo
	Comment 8 • 13 years ago

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?

	Mike Hommey [:glandium]
	Comment 9 • 13 years ago

(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.

	Kyle Huey (Exited; not receiving bugmail, old account, do not use)
	Comment 10 • 13 years ago

So ... can we land this?

	Grant Gayed
	Comment 12 • 13 years ago

(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)?

	Tom Hindle
	Comment 13 • 13 years ago

@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.

	Grant Gayed
	Comment 14 • 13 years ago

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.

	Yun Guo
	Comment 15 • 13 years ago

(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?