Closed Bug 534863 Opened 12 years ago Closed 12 years ago

OOPP: Adobe Reader silent crash (any plugin with spaces in the path)

Categories

(Core :: IPC, defect)

x86
Windows 7
defect
Not set
major

Tracking

()

RESOLVED FIXED

People

(Reporter: jmjjeffery, Assigned: cjones)

References

()

Details

(Whiteboard: [out-of-process only, plugins with spaces in the path (windows)])

Enable the 'IPC' options, restart the browser, then visit a site that has 'pdf's and attempt to open any in the browser.  Minefield will silently crash, with no crash-reporter, or MS error dialog, browser just closes.

No errors noted in Console2.

Using latest hourly build:
Mozilla/5.0 (Windows; U; Windows NT 6.1; en-US; rv:1.9.3a1pre) Gecko/20091215 Minefield/3.7a1pre Firefox/3.6 ID:20091215021820

cset: http://hg.mozilla.org/mozilla-central/rev/c39745ba3c9e

Win7 64bit
Adobe Reader 9.1.0.163

Tested using this page:
http://www.in.gov/judiciary/  
Scroll down and select any PDF file, note after a few seconds the browser closes.
ASSERTION: plugin file ain't there: 'exists', file c:/builds/electrolysis/src/dom/plugins/PluginModuleChild.cpp, line 129

The child crashes in PR_FindFunctionSymbol(PRLibrary* lib = NULL) -> PR_Assert soon after, and then the parent crashes with this stacktrace:

 	xul.dll!NS_DebugBreak_P(unsigned int aSeverity=0x00000003, const char * aStr=0x6c9ea9d0, const char * aExpr=0x00000000, const char * aFile=0x6c9ea980, int aLine=0x00000087)  Line 326	C++
>	xul.dll!mozilla::ipc::AsyncChannel::Close()  Line 135 + 0x18 bytes	C++
 	xul.dll!mozilla::plugins::PPluginModuleParent::Close()  Line 55	C++
 	xul.dll!mozilla::plugins::PluginModuleParent::NP_Shutdown(short * error=0x0031e678)  Line 581	C++
 	xul.dll!mozilla::plugins::PluginModuleParent::~PluginModuleParent()  Line 88	C++
 	xul.dll!mozilla::plugins::PluginModuleParent::`scalar deleting destructor'()  + 0xf bytes	C++
 	xul.dll!nsNPAPIPlugin::~nsNPAPIPlugin()  Line 291 + 0x23 bytes	C++
 	xul.dll!nsNPAPIPlugin::`scalar deleting destructor'()  + 0xf bytes	C++
 	xul.dll!nsNPAPIPlugin::Release()  Line 221 + 0xd6 bytes	C++

which is NS_RUNTIMEABORT("Close() called on closed channel!");

cjones, if you could take the parent channel part of this bug, I'll look at the child-side error.
Assignee: nobody → jones.chris.g
PluginModuleChild::Init aPluginFilename is "C:\Program"

in PluginModuleParent::LoadModule aFilePath is "C:\Program Files (x86)\Adobe\Reader 9.0\Reader\browser\nppdf32.dll"

The bug appears to be that we're calling cmdLine.AppendLooseValue at http://hg.mozilla.org/mozilla-central/annotate/96e8d529b2d3/ipc/glue/GeckoChildProcessHost.cpp#l209 which doesn't quote the argument properly.

in command_line.cc CommandLine::AppendLooseValue is // TODO(evan): quoting?

Perhaps we should be using AppendSwitchWithValue and read the switch in mozilla-runtime instead of a direct argument... switches are quoted correctly.
Blocks: 531142
Pushed http://hg.mozilla.org/projects/electrolysis/rev/69674697dbce
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Im curious as to what bug # the patch for this lives?
The review was fast tracked, done on IRC.  See the hg changeset and commit message.
So, I guess the OOPP stuff is being handled like TraceMonkey, in that the fixes land on electrolysis builds, then at some point merged back into m-c ?  How often are those merges planned, as this, and other OOPP bugs related to plugin's is a deal breaker for some testers and waiting for a long period of time to see the issue fixed on m-c is not good IMO.
You can always unset the ipc pref until we merge again.
(In reply to comment #8)
> You can always unset the ipc pref until we merge again.

This one should be pushed to m-c before the merge, because it appears to be responsible for breaking all other non-flash plug-ins, regardless of the ipc pref setting.
The pushed change doesn't affect in-process plugins.  If they're silently failing it's due to some other problem.
See Benjamin Smedberg's comments in bug 534866
Merged to mozilla-central at http://hg.mozilla.org/mozilla-central/rev/69674697dbce

cjones, could you file followups for this? In particular, the windows-only hack here sucks, because we certainly could end up with "Some Plugin.dylib" on mac/linux. Also, we still need a bug to track the fact that the parent process is crashing (runtimeabort) if the plugin process crashes on load/initialize.
Yeah, like: /Library/Internet Plug-Ins/Flash Player.plugin/ :)
Summary: OOPP: Adobe Reader silent crash of browser when enabled → OOPP: Adobe Reader silent crash (any plugin with spaces in the path)
Whiteboard: [out-of-process only, plugins with spaces in the path (windows)]
(In reply to comment #12)
> In particular, the windows-only hack
> here sucks, because we certainly could end up with "Some Plugin.dylib" on
> mac/linux.

It's not a problem for mac/linux because we use exec() there.

> Also, we still need a bug to track the fact that the parent process
> is crashing (runtimeabort) if the plugin process crashes on load/initialize.

There's bug 535077 for a related issue.  I'll file the bad-|Close()| bug that was causing this abort.
Filed bug 535207 for the failed-init-ABORT.
Flags: in-litmus?
https://litmus.mozilla.org/show_test.cgi?id=11182 has been updated and added to the BFT to cover this scenario
Flags: in-litmus? → in-litmus+
You need to log in before you can comment on or make changes to this bug.