1059504 - Fix crashes in plugin-container due to the new v2 bundle structure.

Status: RESOLVED FIXED

(Core :: IPC, defect)

Description

[Stephen A Pohl [:spohl]]

The new v2 bundle structure causes crashes in plugin-container because four libraries cannot be found by dyld. These libraries are referenced as follows in the plugin-container binary:
@executable_path/libmozglue.dylib
@executable_path/libmozalloc.dylib
@executable_path/libnss3.dylib
@executable_path/XUL

I'm not quite sure how this ever worked, since these libraries never lived next to the plugin-container executable. I do know however that our new bundle structure breaks this and causes plugin-container to crash.

Adding a copy of these libraries in plugin-container.app/Contents/MacOS seems like a very bad idea, since this would dramatically increase the size of the bundle.

There are currently two options I can think of:
1. Go back to a single binary, and do something similar to [1] to avoid a dock icon.
2. Find a way to reference the libraries at @executable_path/../../../<libname>

I understand that by some accident, luck or coincidence, we're currently able to load the necessary libraries from Firefox.app/Contents/MacOS. However, this seems to go against the idea of a .app bundle that could be run independently, since it depends on libraries that are at a precise relative path *outside* of the bundle. For this same reason, I don't like option 2, since we'd still have this dependency. We would just make it explicit.

I'm currently strongly in favor of option 1 but would like to ask a few folks for feedback.

[1] https://bugzilla.mozilla.org/show_bug.cgi?id=557225#c1

Comment 1

[Benjamin Smedberg]

This is an indication that the glue and dependentlibs.list isn't working correctly, and probably a symptom of further problems. I encourage you to step through the glue-loading code to see whether it's looking in the wrong path for dependentlibs.list or the libraries.

Comment 2

[Benjamin Smedberg]

FWIW, a single binary is something I want to do for all platforms, but not in a short timeframe.

Comment 3

[Stephen A Pohl [:spohl]]

Attachment: Patch

You're right (no surprise there! :-)). Although dependentlibs.list and the libraries got loaded correctly, I failed to adjust a subsequent lookup of the GREDir. This fixes all the crashes that I've observed and I've also inspected the rest of GeckoChildProcessHost.cpp to ensure that we're doing the proper thing.

This didn't turn out as bad as I thought. I still don't like the idea of referencing libraries that live outside of the .app bundle, but strictly speaking, if we wanted to use the plugin-container.app bundle separately, all we would need to do is add a copy of those four libraries to plugin-container.app/Contents/MacOS.

Thanks for saving me a lot of time, Benjamin!

Comment 4

[Stephen A Pohl [:spohl]]

Comment on attachment 8480295 [details] [diff] [review]
Patch

Try push is almost completely green, so requesting review (updater xpcshell test failures and gaia python integration test suite failures are handled in separate bugs):
https://tbpl.mozilla.org/?tree=Try&rev=7362867ad903

Comment 5

[Benjamin Smedberg]

Comment on attachment 8480295 [details] [diff] [review]
Patch

I don't think XRE_GetFileFromPath is correct in this context. `path` is not relative nor argv[0]. I'm pretty sure you want NS_NewLocalFile(gGREPath).

Comment 6

[Stephen A Pohl [:spohl]]

Attachment: Patch

Sent this to Oak with all the other patches. Waiting for the results before requesting review again:
https://tbpl.mozilla.org/?tree=Oak&rev=45d91d73dbf5

Comment 7

[Benjamin Smedberg]

Comment on attachment 8488816 [details] [diff] [review]
Patch

Not sure you really want `true` for followLinks here. If it works without, false is probably the safer option.

Comment 8

[Stephen A Pohl [:spohl]]

Attachment: Patch

Thanks, Benjamin! I wasn't sure which option was safer. Changed followLinks to false.

Comment 9

[Stephen A Pohl [:spohl]]

https://hg.mozilla.org/projects/oak/rev/f56689179777

Comment 10

[Robert Strong (they/them - no direct email)]

Pushed to fx-team
https://hg.mozilla.org/integration/fx-team/rev/cbd3f8e4bf49

Comment 11

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/cbd3f8e4bf49

Comment 12

[Robert Strong (they/them - no direct email)]

Landed on aurora in the Mac V2 signing combined patch in bug 1047584