889480 - If NP_Initialize fails NP_Shutdown is still executed

Status: RESOLVED FIXED

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

Description

[Gabor Krizsanits (INACTIVE)]

https://bugzilla.mozilla.org/show_bug.cgi?id=768651#c17

So in case of error in the init, the plugin is destroyed, and since the plugin functions are set at this point on windows, from the destructor chain we call NP_Shutdown on the uninitialized plugin.  

Would it be a reasonable fix for this bug to set mShutdown to true in PluginModuleParent::NP_Initialize (windows version) in case of error?

relevant call stack for the shutdown call:

>	xul.dll!mozilla::plugins::PPluginModuleParent::CallNP_Shutdown(short * rv=0x0047ecb4) Line 425	C++
 	xul.dll!mozilla::plugins::PluginModuleParent::NP_Shutdown(short * error=0x0047ecb4) Line 1257	C++
 	xul.dll!mozilla::plugins::PluginModuleParent::~PluginModuleParent() Line 164	C++
 	xul.dll!mozilla::plugins::PluginModuleParent::`scalar deleting destructor'(unsigned int)	C++
 	xul.dll!nsNPAPIPlugin::~nsNPAPIPlugin() Line 244	C++
 	xul.dll!nsNPAPIPlugin::`scalar deleting destructor'(unsigned int)	C++
 	xul.dll!nsNPAPIPlugin::Release() Line 231	C++
 	xul.dll!nsRefPtr<nsNPAPIPlugin>::~nsRefPtr<nsNPAPIPlugin>() Line 881	C++
 	xul.dll!nsNPAPIPlugin::CreatePlugin(nsPluginTag * aPluginTag=0x0c2582d8, nsNPAPIPlugin * * aResult=0x0047ee50) Line 445	C++
 	xul.dll!CreateNPAPIPlugin(nsPluginTag * aPluginTag=0x0c2582d8, nsNPAPIPlugin * * aOutNPAPIPlugin=0x0047ee50) Line 1422	C++

Comment 1

[Benjamin Smedberg]

Attachment: When NP_Initialize fails, we should not try to call NP_Shutdown later, r?gfritzsche

Automated testing of this would be a royal pain for less-than-normal gain, so I'd like to just not do that.

Comment 2

[Georg Fritzsche [:gfritzsche]]

Comment on attachment 770317 [details] [diff] [review]
When NP_Initialize fails, we should not try to call NP_Shutdown later, r?gfritzsche

Review of attachment 770317 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good to me and i can't think of a short-cut for testing either.

Comment 3

[Ryan VanderMeulen [:RyanVM]]

Something in this push was causing mochitest-bc failures. Backed out.
http://hg.mozilla.org/integration/mozilla-inbound/rev/e6d3454da7a7

https://tbpl.mozilla.org/php/getParsedLog.php?id=25380390&tree=Mozilla-Inbound

Comment 4

[Ryan VanderMeulen [:RyanVM]]

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