517923 - Electrolysis: PluginModuleChild::_geturl crashes on NULL char*, needs to serialize them

Status: RESOLVED FIXED

(Core Graveyard :: Plug-ins, defect, P1)

Description

[Benjamin Smedberg]

I was trying to run the plugin unit tests on Electrolysis. If the plugin is unloaded and then reloaded, the plugin process is defunct: we try to send a message to it which hangs forever.

Related to bug 516509 but it should be possible to deal with these lifetime issues while keeping the magic/awful singleton.

Comment 1

[Benjamin Smedberg]

Steps to reproduce:

* un-do http://hg.mozilla.org/projects/electrolysis/rev/e81269651691 so that mochitests turn on plugin-IPC (rebuild testing/mochitest)
* cd objdir/_tests/testing/mochitest
* python runtests.py --test-path=modules/plugin/test/test_pluginstream.html

This test instantiates the plugin multiple times (in order to do variations on the test). At the second instantiation the first mozilla-runtime process is defunct but we're trying to send it a message.

Comment 2

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

Slightly simpler (for me) is to follow these instructions (https://developer.mozilla.org/en/Mochitest#Running_individual_tests_in_different_Firefox_sessions) and then load http://localhost:8888/tests/modules/plugin/test/test_pluginstream.html in the child session.

Comment 3

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

This particular crash points at a bug in the XPCOM string code, bug 518001.

But the underlying problem is more general, which is that NULL is sometimes meaningful for NPAPI interfaces (NPN_GetURL(), e.g.) and we need to serialize NULL.  The tentative plan is to use "void" ns*String*s to represent NULL.

Comment 4

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

Pushed http://hg.mozilla.org/projects/electrolysis/rev/1dad436eaa99

I should note that this only fixes the hang/crasher.  The first stage of the fix *without* NullableString*() helpers in PluginInstanceParent.cpp would display the "Lorem ipsum ..." text.  The second version that added those helpers no longer did.  I'm not sure where the bug there lies, or if it even exists, so ping-ponging it back to those knowledgeable about NPStream.

Comment 5

[Benjamin Smedberg]

hrm, I'm not sure I like this solution, but it will do for now. Now to debug the failure...

Comment 6

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

Can you be more specific?  Any solution to this problem needs to distinguish NULL and "", and hence serialize both, as the two values have different meaning to some NPAPI functions.

Comment 7

[Benjamin Smedberg]

Yes, but the notion that XPCOM strings have a special void state is awful and we've been trying to get rid of it for years. Now we're propagating it to new realms.

Comment 8

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

That's fair, IsVoid seems wonky to me as well.

I'd be all for creating mozilla::plugins::NullableString as a wrapper around nsCString to do what we're abusing nsCString::IsVoid for.