Significant memory leak associated with javascript-mapped plugin function calls

[GoogleCodeExporter]

What steps will reproduce the problem?
1. build and register the latest FBTestPlugin dll (1.4)
2. Open the attached html file in your browser
3. Start up process explorer or task manager and monitor browser memory use
4. Click "Start the leak"

What is the expected output? What do you see instead?

Ideally, there should be no change in the memory footprint of the browser. 
Instead, it rises linearly in time, about 1 meg every 5 or 10 seconds.

What version of FireBreath are you using? On what operating system and
browsers?

FireBreath v1.4. Windows 7, 64 bit, Chrome 9.0, Firefox 3.6 and IE8

What version of your compiler or IDE are you using?

Visual Studio 2008.

Please provide any additional information below.

Properties are not affected by the leak.

Original issue reported on code.google.com by gearoid....@gmail.com on 28 Feb 2011 at 10:31

Attachments:

• version
• leakTest.htm

[GoogleCodeExporter]

Found to be invalid

Original comment by taxilian on 2 Mar 2011 at 2:36

• Changed state: Invalid

[GoogleCodeExporter]

What does an invalid bug mean?, were you unable to reproduce the memory leak or 
is the origin of the leak beyond the scope of the plugin?

Original comment by gearoid....@gmail.com on 2 Mar 2011 at 7:00

[GoogleCodeExporter]

I apologize; I read this and thought it was a different issue.  Wasn't paying 
complete attention, I guess.

However, I do not believe that this is actually a leak; rather, it's a combined 
result of browser inefficiency in garbage collection and an inefficiency of 
FireBreath which is fixed in 1.5.  However, since you're only using the root 
JSAPI object, this is most likely mostly on the browser side.

Keep in mind that the function in question does not return the same object each 
time; getUserData() creates a new javscript object and returns it each time the 
call is made, so if the browser doesn't do garbage collection often enough that 
will add up.

In short, I don't think there is anything we can do about this, but I welcome 
your comments or suggestions.

Original comment by rich...@firebreath.org on 2 Mar 2011 at 7:48

• Changed state: New

[GoogleCodeExporter]

Thanks for getting back to me.

One of our products deploys ssh tunnels for mission critical support
infrastructure via browser plugins (a la FireBreath). It needs to run for
weeks without fail, which is why this leak is something of a priority for
us. Our current plan is to remove as many function calls as we possibly can
and map their functionality onto registered properties, as the properties do
not seem to be affected.

I also suspect the browser as being the sole cause of the leak as I modified
"NPJavascriptObject::_Invoke" to simply return true (instead of propagating
the call down to the plugin) and the leak remained unabated. If it is simply
a case of laggard garbage collection in the browser then I can test for that
easily and get back to you.

I might also try watching NPN_ReleaseVariantValue in the debugger to verify
that these objects are getting released.

Original comment by gearoid....@gmail.com on 2 Mar 2011 at 8:17

[GoogleCodeExporter]

Sounds pretty cool =] would be awesome to see that on the "FireBreath Users" 
page.

You could try updating to 1.5 (in the master branch). It's still pretty solid, 
but it has a few fixes in it that may help with this issue; one of which is the 
ability to cache NPObjects, etc, so that it doesn't create a new one each time 
it returns a JSAPI object.

Are you experiencing this issue any time you call any function or only on 
functions returning a VariantList or VariantMap?  The main reason that the code 
you submitted is so bad is that it creates a javascript object as the return 
value each time.  You could cache that object, if it doesn't need to be unique 
each time, and that would probably "fix" the problem.

Feel free to drop into the IRC room (http://npapi.com/chat) and discuss this 
with me sometime; I may be able to help, but I need to know more about how 
you're using the API.

Original comment by rich...@firebreath.org on 2 Mar 2011 at 8:22

[GoogleCodeExporter]

I know for a fact that we couldn't have provided this product had it not
been for FireBreath, I'd have to run it by my managers first but I'm sure
they wouldn't mind too much if we added our story to the "FireBreath Users"
page.

Now that you mention it, the leak does seem to be associated with list and
map structures. When you say cache the object, do you mean, creating the
object once in the plugin C++ space and returning it by reference each time?

Original comment by gearoid....@gmail.com on 2 Mar 2011 at 8:32

[GoogleCodeExporter]

sorry, I lose track of things sometimes, so I got doing other things and forgot 
I hadn't answered.  Yes, that's exactly what I mean.  As long as you don't need 
to return by reference, you can get an array (or list) from javascript using 
getDOMWindow->createArray and createMap

http://npapi.com/x/p48T for relevant docs

Can we consider this issue closed for now, then? sounds like yes, though it 
would be nice if we could find a way to improve this.

Original comment by taxilian on 4 Mar 2011 at 7:10

[GoogleCodeExporter]

No worries :) we're all busy.

I've been away from work for the last few days, so I won't be able to verify
your suggestions until Monday.

If this solves the problem, I'll be happy but I think others could
unknowingly run into the same issue in the future, it would be nice to have
an another plugin example which explicitly demonstrated all potential memory
leaks and how to get around them, especially the ones which are beyond the
control of the plugin, I wouldn't mind writing up such a simple example when
I get time.

It just seems improbable to me that the NPAPI doesn't have a cleanup
strategy for these NPObjects. When I get time, I'd like to pursue this
further, if only to satisfy my curiosity.

Original comment by gearoid....@gmail.com on 4 Mar 2011 at 11:52

[GoogleCodeExporter]

Ok, I had a chance to verify your suggestions today. The VariantList and 
VariantMap structures are certainly the origin of the problem, regardless of 
the browser, regardless of how the data is acquired (ie, via the return value 
of a mapped javascript function or via a property method). Strings, ints, float 
and other similar types are not affected.

It's worth noting that the memory is freed once the browser is refreshed. This 
suggests to me that the memory isn't leaking (in the traditional sense) but is 
not being released appropriately. I've eyeballed the reference counts of the 
objects as they're passed back into the browser and they seem fine so I'm a 
little perplexed as to why this is happening. The fact that it's consistent 
across all the browsers indicates that the problem may lie with FireBreath.

Original comment by gearoid....@gmail.com on 7 Mar 2011 at 9:42

[GoogleCodeExporter]

For the sake of argument, try calling host->DoDeferredRelease() in whatever way 
is convenient after the memory leak has built up a bit.  That is the only place 
we can sometimes hold onto the references ourselves, but if the firebreath 
wrapper for the object is released on the main thread it should release it 
immediately.

Still, worth a try.

Original comment by richarda...@gmail.com on 7 Mar 2011 at 9:55

[GoogleCodeExporter]

Ok, I'll give it a shot.

On a related note, I went digging around on Google Code Search for
"NPObject" and "referenceCount" to see if I found anything interesting and
it lead me back to NPObjectAPI.cpp. There's a comment in the
destructor of NPObjectAPI
along these lines:

"
        Okay, this is far from perfect, but if the reference count
         * is less then RETAIN_COUNT or is higher than
         * RETAIN_COUNT * REFCOUNT_CUTTOFF_MULT then something
         * is really off; to be safe, we'll just avoid doing any releases on
         * this object; this will absolutely cause a memory leak; however,
         * that's better than a crash.
"

I don't know the firebreath code well enough but it seems that the problem
we're observing could be related to the cautious referenceCount treatment in
the NPObjectAPI destructor?

Original comment by gearoid....@gmail.com on 7 Mar 2011 at 10:01

[GoogleCodeExporter]

Argh!, my bad, I just checked the latest code, I must have been looking at some 
cached code :(

Original comment by gearoid....@gmail.com on 7 Mar 2011 at 10:10

[GoogleCodeExporter]

Yeah, we used to use google code/mercurial for source control, so that can 
happen. We found and fixed this issue awhile back.

Original comment by richarda...@gmail.com on 8 Mar 2011 at 1:43

[GoogleCodeExporter]

Ok, I've done some tests this morning, the leak is associated with NPObjects 
and IE. All the other browsers show memory footprint increases as I pummel them 
with the javascript snippet but they all clean up after themselves eventually.

IE is generally well behaved if a lot slower to apply garbage collection when 
the variant type is int, float or string. When the variant type encapsulates an 
NPObject, the leak manifests in IE: Memory footprint steadily increases and 
stays constant if the leak loop is stopped. I would speculatively attribute 
this to the ActiveX wrapper required by IE to make these plugins work.

Original comment by gearoid....@gmail.com on 9 Mar 2011 at 11:47

[GoogleCodeExporter]

NPObjects aren't used on IE; IDispatch is. It could be that the reference 
counting in IE is such that it doesn't nicely / easily release things.

Is there an upper limit to the memory usage?  Did you ever try calling 
DoDeferredRelease on the browserhost like I suggested?

Original comment by taxilian on 9 Mar 2011 at 6:01

[GoogleCodeExporter]

I haven't actually tried to see if there's a limit to how much memory can be
consumed, my guess is that the limits are related to the process memory
limits for the platform (ie 32 vs 64 bit Linux and Windows (XP, Vista and
7)). I'll do a quick test to see if that's correct.

Haven't tried DoDeferredRelease yet, will get to it tomorrow.

Original comment by gearoid....@gmail.com on 9 Mar 2011 at 6:54

[GoogleCodeExporter]

Moving this issue to JIRA: http://jira.firebreath.org/browse/FIREBREATH-3

Please continue the discussion there and let me know what you've found =]

Original comment by taxilian on 14 Mar 2011 at 7:03

• Changed state: Done