501930 - missing release after QI in nsJavaWrapper.cpp

Status: RESOLVED WONTFIX

(Core Graveyard :: Java to XPCOM Bridge, defect)

Description

[Rick Vestal]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 6.1; en-US) AppleWebKit/530.5 (KHTML, like Gecko) Chrome/2.0.172.33 Safari/530.5
Build Identifier: 3.0

Around line 750, the following code exists:

        rv = JavaObjectToNativeInterface(env, java_obj, iid, &xpcom_obj);
        if (NS_FAILED(rv))
          break;
        rv = ((nsISupports*) xpcom_obj)->QueryInterface(iid, &xpcom_obj);
        if (NS_FAILED(rv))
          break;

The QI addref's the xpcom_obj, but it is never released.  Changing to the following gets rid of the extra addref:

        rv = JavaObjectToNativeInterface(env, java_obj, iid, &xpcom_obj);
        if (NS_FAILED(rv))
          break;
        NS_ENSURE_TRUE( xpcom_obj, NS_ERROR_FAILURE );
        
        nsISupports *localXPCOM = (nsISupports*) xpcom_obj;
        rv = localXPCOM->QueryInterface(iid, &xpcom_obj);
        NS_IF_RELEASE( localXPCOM );

        if (NS_FAILED(rv))
          break;


Reproducible: Always

Comment 1

[David]

I have detected code that doesn't leak in Javascript, whereas it leaks in Java.
This problem could be the cause.

Comment 2

[David]

Confirmed that my memory leaks have improved after having applied this patch.
Probably there are more problems.
Thanks a lot Rick.

Comment 3

[Gerald]

Attachment: Patch for Rick Vestal fix

Benjamin, I created now a patch from ricks fix (see comment 1). 
I kindly ask you to review this patch, it contains only xpcom related changes and was tested by three different people (rick, david, and me). It showed that we have less memory leaks with this patch.

In case you approve it, can you please also commit it, as none of us here has commit rights. Maybe also for 1.9.2 or even 1.9.1 branch or is it for this branches already to late to check it in?

Thx in advance.

Comment 4

[Benjamin Smedberg]

Comment on attachment 396130 [details] [diff] [review]
Patch for Rick Vestal fix

there should be at least no tabs in this file (which makes it hard to review as well).

But I don't really understand this code and am not a good reviewer. The module owner (jhpedemonte) should review if he's still active.

Comment 5

[Mike Hommey [:glandium] OoO until Jun 18]

JavaXPCOM was removed in bug 648593.