Closed Bug 542053 Opened 14 years ago Closed 14 years ago

OOPP do not work in XR builds

Categories

(Core :: IPC, defect)

x86
macOS
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla1.9.3a4
Tracking Status
blocking1.9.2 --- needed
status1.9.2 --- .4-fixed

People

(Reporter: dougt, Assigned: cjones)

References

Details

Attachments

(2 files)

Attached patch patch v.1 (WIP)Splinter Review
http://mxr.mozilla.org/mozilla-central/source/ipc/glue/GeckoChildProcessHost.cpp#182

We current assume that the mozilla-runtime lives next to the executable (eg. firefox-bin).

There are some configurations, like fennec, that use separate directory for their application and the mozilla runtime stuff.  For this case, we want to do something like look up the XRE location using the directory service.
this isn't a complete solution because LD_LIBRARY_PATH still needs to be set in order to avoid missing symbols:

/home/user/fennec/xulrunner/mozilla-runtime: symbol lookup error: /home/user/fennec/xulrunner/mozilla-runtime: undefined symbol: XRE_StringToChildProcessType
Attachment #423400 - Attachment is patch: true
Attachment #423400 - Attachment mime type: application/octet-stream → text/plain
Attachment #423400 - Flags: review?(benjamin)
Attachment #423400 - Flags: review?(benjamin) → review+
Blocks: 542784, OOPP
Assignee: nobody → dougt
Target Milestone: --- → mozilla1.9.3a4
Doug, are you going to implement the other half of this bug (setting up LD_LIBRARY_PATH programmatically)?
Wouldn't a $ORIGIN rpath be better ?
I really don't want to muck about changing rpath stuff on the stable branch: the OOPP code is planned to backport to 1.9.2/Fx3.6. I would welcome an investigation of using ORIGIN on trunk for 1.9.3, if we can rely on a new-enough ld.so... when was support for origin added to ld.so?
(In reply to comment #5)
> I really don't want to muck about changing rpath stuff on the stable branch:
> the OOPP code is planned to backport to 1.9.2/Fx3.6. I would welcome an
> investigation of using ORIGIN on trunk for 1.9.3, if we can rely on a
> new-enough ld.so... when was support for origin added to ld.so?

According to glibc's git, version 2.0.96, released in 1998.
cjones, can you hack in the environment-variable bit? This is going to bite Linux distros for 3.6.4 unless they disable OOPP, so a hack which we can land today is better than a perfect solution.
Assignee: dougt → jones.chris.g
blocking1.9.2: --- → ?
Not tested in a xulrunner build, but it'd stomping my existing LD_LIBRARY_PATH with the right thing for my normal build.
Attachment #438148 - Flags: review?(benjamin)
Comment on attachment 438148 [details] [diff] [review]
Set LD_LIBRARY_PATH=[GRE dir] for mozilla-runtime

Looks good. Let's get this landed in m-c and mh/wrosenauer can I get one of you to verify?
Attachment #438148 - Flags: review?(benjamin) → review+
I'm a bit confused. I've built from 1.9.2 branch today before and created testpackages with usual xulrunner/firefox split and w/o doing anything special I got a working mozilla-runtime based Flash applet. Not quite sure why it just works for me currently.
Whiteboard: [land-lorentz]
Based on comment 11 and the red builds after checkin, I think we'll wait for this to bake a little longer!
blocking1.9.2: ? → needed
The red is just a consequence of needless platform-specific fragmentation in some of this code, and me thinking this patch was small enough not to tryserver.  No cause for alarm.

Followups
http://hg.mozilla.org/mozilla-central/rev/c11e93ca14b6
http://hg.mozilla.org/mozilla-central/rev/88a9ffdc4fea
(In reply to comment #8)
> Created an attachment (id=438148) [details]
> Set LD_LIBRARY_PATH=[GRE dir] for mozilla-runtime

You should keep whatever was set before and append :$GRE_DIR if there was a value. If for $whatever reason the previous LD_LIBRARY_PATH was necessary to get some required libraries, it won't work.
(In reply to comment #11)
> I'm a bit confused. I've built from 1.9.2 branch today before and created
> testpackages with usual xulrunner/firefox split and w/o doing anything special
> I got a working mozilla-runtime based Flash applet. Not quite sure why it just
> works for me currently.

Are you using run-mozilla.sh ? In that case, LD_LIBRARY_PATH is already set.
(In reply to comment #15)
> (In reply to comment #11)
> > I'm a bit confused. I've built from 1.9.2 branch today before and created
> > testpackages with usual xulrunner/firefox split and w/o doing anything special
> > I got a working mozilla-runtime based Flash applet. Not quite sure why it just
> > works for me currently.
> 
> Are you using run-mozilla.sh ? In that case, LD_LIBRARY_PATH is already set.

No, I'm using xulrunner-stub without any environment LD hacks.
But I guess it works for me because I set -rpath for all the xulrunner components (including mozilla-runtime) to the GRE dir.
I do that since ages and it should be fine since all stuff from one GRE should first depend on itself.
(In reply to comment #14)
> (In reply to comment #8)
> > Created an attachment (id=438148) [details] [details]
> > Set LD_LIBRARY_PATH=[GRE dir] for mozilla-runtime
> 
> You should keep whatever was set before and append :$GRE_DIR if there was a
> value.

No.  If you're referring to release builds, please supply a compelling use-case that applies to people other than Mozilla/Gecko/xulrunner developers for non-release builds.
(In reply to comment #17)
> No.  If you're referring to release builds, please supply a compelling use-case
> that applies to people other than Mozilla/Gecko/xulrunner developers for
> non-release builds.

Some Linux users have a tendency to have weird setups, and I wouldn't be surprised some of them have some libraries in some weird directory they put in LD_LIBRARY_PATH. Some of these libraries could very well be dependencies of the XRE. I, for one, though not in recent days, have sometimes used a $HOME/lib directory to install newer versions of some libraries. If the XRE is using these, you may want mozilla-runtime to do so as well.
This will never get branch approval until it's marked FIXED...
Status: NEW → RESOLVED
Closed: 14 years ago
Component: Plug-ins → IPC
QA Contact: plugins → ipc
Resolution: --- → FIXED
Comment on attachment 438148 [details] [diff] [review]
Set LD_LIBRARY_PATH=[GRE dir] for mozilla-runtime

a=LegNeato for 1.9.2.4
Attachment #438148 - Flags: approval1.9.2.4? → approval1.9.2.4+
So, I guess I'll have to file a new bug for comment 18...
FWIW, a similar issue was reported in bug 434997 comment 5, but we don't seem to have a bug report on that.

It would be nice to sort something out to make it easier to run child processes against debug libraries, though.

I wonder whether -Wl,-rpath,'$ORIGIN' would be a sensible solution.
(In reply to comment #24)
> I wonder whether -Wl,-rpath,'$ORIGIN' would be a sensible solution.

It would just do the right thing, wouldn't require the patch from this bug, and would handle LD_LIBRARY_PATH properly. And cf. comment 6, it's been supported for ages.
bug 552864 covers ditching the wrapper scripts in favor of -rpath=$ORIGIN.
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: