41880 - MRJ plugin causes 'service manager not shutdown' assertion on Quit

Status: RESOLVED INCOMPLETE

(Core Graveyard :: Java: OJI, defect, P3)

Description

[Simon Fraser [no longer active]]

If I have the MRJ plugin in the plugins folder, I get an assertion from the 
service manager that it was not properly shut down on quit. This causes all 
services to be leaked.

Comment 1

[Patrick C. Beard]

Reassigning to OJI.

Comment 2

[Simon Fraser [no longer active]]

Any progress on this bug?

Comment 3

[drapeau]

No resources to handle Macintosh bugs right now.  Changing Target Milestone to 
reflect this.

Comment 4

[Simon Fraser [no longer active]]

This sounds bad. CCing some mozilla folks.

Comment 5

[drapeau]

Changing milestone to undefined to reflect that there are no resources to work
on Macintosh bugs.

Comment 6

[Simon Fraser [no longer active]]

Does this mean that we have no Java support on Mac? What is the status of Java in 
Mac Mozilla?

Comment 7

[drapeau]

Yep, this means no Java support on the Mac right now.  I have a req open to hire 
somebody, but I personally don't have any people with Macintosh experience.  I 
apologize for this, and am trying to correct the problem by going to job fairs 
and doing more to find Mac expertise to hire.  I'd be happy to help external 
contributors help out; if you know of any, send 'em my way.

Comment 8

[Peter Van der Beken]

I think the problem is that the MRJPlugin holds on to the Service Manager 
(http://lxr.mozilla.org/seamonkey/source/plugin/oji/MRJ/plugin/Source/
MRJPlugin.cpp#81). It should release the service manager as part of it's 
Shutdown method (http://lxr.mozilla.org/seamonkey/source/plugin/oji/MRJ/plugin/﷒0﷓). Now Shutdown should be called from 
nsPluginHostImpl::Destroy (http://lxr.mozilla.org/seamonkey/source/modules/
plugin/nglsrc/nsPluginHostImpl.cpp#2019) when the plugin service gets destroyed. 
And the plugin service gets destroyed when the service manager releases it, as 
part of its shutdown. But because MRJPlugin holds on to the service manager, the 
service manager doesn't shut down properly. At least that's what I think is 
happening from debugging and looking at the source. As to how to fix it, I don't 
know. This patch causes the service manager to shutdown properly, but is not the 
right thing to do:

Index: mozilla/plugin/oji/MRJ/plugin/Source/MRJPlugin.cpp
===================================================================
RCS file: /cvsroot/mozilla/plugin/oji/MRJ/plugin/Source/MRJPlugin.cpp,v
retrieving revision 1.30
diff -u -4 -r1.30 MRJPlugin.cpp
--- MRJPlugin.cpp	2000/10/28 05:55:33	1.30
+++ MRJPlugin.cpp	2000/10/31 19:32:46
@@ -80,8 +80,9 @@
 
 	if (theServiceManager == NULL) {
 		if (serviceManager->QueryInterface(NS_GET_IID(nsIServiceManager), &
theServiceManager) != NS_OK)
 			return NS_ERROR_FAILURE;
+        theServiceManager->Release();
 
 		// Our global operator new wants to use nsIMalloc to do all of its 
allocation.
 		// This should be available from the Service Manager.
 		if (theServiceManager->GetService(kMemoryCID, NS_GET_IID(nsIMemory), 
(nsISupports**)&theMemoryAllocator) != NS_OK)
@@ -299,9 +300,9 @@
     }
 
     // release our reference to the service manager.
     if (theServiceManager != NULL) {
-        theServiceManager->Release();
+        //theServiceManager->Release();
         theServiceManager = NULL;
     }
     
     return NS_OK;

Comment 9

[shrirang khanzode]

qa:junruh

Comment 10

[Patrick C. Beard]

I've already checked in changes for this, but the cycle can't be broken yet 
because Shutdown() never gets called. See bug #49510 for more information.

Comment 11

[Peter Van der Beken]

Hmm, but I can make it call Shutdown by breaking the circular dependency between 
MRJPlugin and the plugin service. Right now, MRJPlugin holds on to the service 
manager in 'theServiceManager' and to the plugin service in 'thePluginManager' 
and 'thePluginManager2'. It releases these references in the Shutdown method. If 
it releases them earlier, which is probably dangerous as it uses them later on, 
then the cycle is broken, the service manager shuts down properly and the plugin 
service gets destroyed properly, calling Shutdown on the MRJPlugin. Try it out 
by releasing 'theServiceManager' in NSGetFactory and releasing 
'thePluginManager' and 'thePluginManager2' in the MRJPlugin::Initialize. This is 
(again) not the right solution but it shows that there is a circular dependency 
that will need to be removed somehow.

Comment 12

[drapeau]

Re-assigning to Terry Noyes.

Comment 13

[Patrick C. Beard]

The service manager is needed at arbitrary times, beyond initialization, so it 
can't be released early. We need an explicit call to each nsIPlugin's shutdown 
method, before shutting down the service manager, to break cycles. Either that, 
or a weak reference needs to be held to the service manager, which would require 
some work in XPCOM to pull off.

Comment 14

[Peter Lubczynski]

I think this may be more general, it seems to happen with other plugins as well 
like Shockwave.

Comment 15

[Peter Lubczynski]

Nominating. This happens with the Sun plugin too.

Patrick, nsIPlugin::Shutdown should now be called at shutdown so perhaps Peter's 
fix? nsPluginHostImpl has xpcom-shutdown Observers now. 

Comment 16

[Peter Lubczynski]

I think the problem may be releasing the service manager in Shutdown(). 
Shutdown() is only called if there is a matching Initilize(). The can cause a 
problem in the case of just bringing up the browser then closing it.

Why doesn't the plugin just hold a weak ref instead?

Comment 17

[John Unruh]

Setting target to future.

Comment 18

[Patty Mac]

Chris Petersen is a new QA contact for oji component. His email is:
[e-mail]

Comment 19

[Matthias Versen [:Matti]]

fixing small error for [e-mail] (filter with : SPAMMAILSUCKS)

Comment 20

[Simon Paquet [:sipaq]]

This bug is targeted at a Mac classic platform/OS, which is no longer supported
by mozilla.org. Please re-target it to another platform/OS if this bug applies
there as well or resolve this bug.

I will resolve this bug as WONTFIX in four weeks if no action has been taken.
To filter this and similar messages out, please filter for "mac_cla_reorg".

Comment 21

[Bob Clary [:bc] (inactive)]

-> default assignee for old netscape assigned bugs.

Comment 22

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

I'm not seeing this with recent JEPs; if no one else (Simon, Steven) is seeing this, please WFM it.

Comment 23

[Benjamin Smedberg]

Mass-closing bugs in the "OJI" component: OJI plugin integration was replaced with npruntime long ago, and these bugs appear to be irrelevant now. If there is in fact a real bug that remains, please file it new in the "Core" product, component "Plug-ins".