nsIPlugin::Shutdown is not called if there is no nsIPluginInstance object created

VERIFIED FIXED in mozilla0.9

Status

()

P2
normal
VERIFIED FIXED
18 years ago
18 years ago

People

(Reporter: serhunt, Assigned: serhunt)

Tracking

Trunk
mozilla0.9
x86
All
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

18 years ago
It is possible that after general plugin initialization (nsIPlugin 
stuff) nsIPluginInstance object is never created. We should still call 
nsIPlugin::Shutdown method when we clean up. MRJ is an example of such a plugin.
(Assignee)

Updated

18 years ago
Status: NEW → ASSIGNED
Priority: -- → P2
Target Milestone: --- → mozilla0.9
(Assignee)

Comment 1

18 years ago
--> 0.9
(Assignee)

Comment 2

18 years ago
Patric, would you please take a look and if this patch will address what we 
talked about the other day?

NS_IMETHODIMP nsPluginHostImpl::Destroy(void)
{
  if (mIsDestroyed)
    return NS_OK;

  mIsDestroyed = PR_TRUE;

-  // at this point nsIPlugin::Shutdown calls will be performed if needed
+  // at this point nsIPlugin::Shutdown calls will be performed on last inst  
  mActivePluginList.shut();

+  //go through the list of the plugins to see if we have anything to shutdown
+  for(nsPluginTag * p = mPlugins; p != nsnull; p = p->mNext)
+  {
+    if(p->mEntryPoint != nsnull)
+    {
+      p->mEntryPoint->Shutdown();
+      p->mEntryPoint = nsnull;
+    }
+
+    if ((nsnull != p->mLibrary) && p->mCanUnloadLibrary)
+    {
+      PR_UnloadLibrary(p->mLibrary);
+      p->mLibrary = nsnull;
+    }
+  }

  return NS_OK;
}

Comment 3

18 years ago
I think I saw another bug like this. Is this patch going in?
(Assignee)

Comment 4

18 years ago
I filed this bug after Patric pointed out this problem to me. I still want to 
see his response if the solution above will help.

Comment 5

18 years ago
Patch appears to work. r=beard

Comment 6

18 years ago
One point to consider, do we know that all existing plugin instances have been 
deleted at this point? If so, then this is what we've been needing all along.
(Assignee)

Comment 7

18 years ago
Yes, mActivePluginList.shut(); destroys all the instances. But as far as I 
understand from what we talked about theoretically a plugin (nsIPlugin) may not 
even create a single instance (nsIPluginInstance) but still needs to be 
shutdown. This patch addresses this specific situation.

Comment 8

18 years ago
sr=waterson
(Assignee)

Comment 9

18 years ago
There is a loop now in nsPluginHostImpl::Destroy to delete members of the 
plugin list (bug 73289) so we can move this shutdown stuff in there. No reason 
to have a separate loop for this.
(Assignee)

Comment 11

18 years ago
cheked in
Status: ASSIGNED → RESOLVED
Last Resolved: 18 years ago
Resolution: --- → FIXED

Comment 12

18 years ago
verified patch is in the tree.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.