55821 - MLK: nsPluginHostImpl::ScanPluginsDirectory()

Status: VERIFIED FIXED

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

Description

[Bruce Mitchener]

In nsPluginHostImpl::ScanPluginsDirectory(), it does:

nsPluginInfo info = { sizeof(info) };
nsresult res = pluginFile.GetPluginInfo(info);

At least on Unix, the GetPluginInfo method allocates memory, which is not 
subsequently freed in this code.

The info struct is passed into the constructor of a nsPluginTag, which copies 
the memory.

Cc'ing valeski so that someone who is more on top of leaks than I am currently 
is aware of it.

Comment 1

[av (gone)]

bruce wrote:
>The info struct is passed into the constructor of a nsPluginTag, which copies 
>the memory.

After which pluginFile.FreePluginInfo(info) is called. Are you saying it does 
not do the job on Unix?

Comment 2

[Bruce Mitchener]

Thanks for making me look at this closer. I should've seen this the first time. 
:)

The 3 allocations that leak are the lines:

219     info.fMimeTypeArray =(char **)PR_Malloc(num * sizeof(char *));
220     info.fMimeDescriptionArray =(char **)PR_Malloc(num * sizeof(char *));
221     info.fExtensionArray =(char **)PR_Malloc(num * sizeof(char *));

The FreePluginInfo() method does indeed free up most of the memory, but it fails 
to free the actual arrays themselves. (It handles their contents just fine.)

Looking at all of the other nsPluginFile implementations, this leak does not 
happen on OS/2, MacOS, or Windows.  I feel ill after just looking at the BeOS 
implementation (it doesn't deallocate anything!).

So, this looks to be unix only, and a separate bug ought to be filed on the BeOS 
people's implementation since it is a larger problem.  I'll file that one now.

To fix this, you just need something like:
PR_Free(info.fMimeTypeArray);
PR_Free(info.fMimeDescriptionArray);
PR_Free(info.fExtensionArray);

after the for loop.

Comment 3

[ekrock's old account (dead)]

Not a Netscape 6 RTM blocker. FUTURE. This bug has been marked Future because
the Netscape engineer it is assigned to is overburdened.

Comment 4

[av (gone)]

Looks like it is fixed now.

Comment 5

[shrirang khanzode]

marking verified.