317012 - Crash in Release() in nsJavaXPTCStub::FinalizeJavaParams()

Status: RESOLVED FIXED

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

Description

[jhp (no longer active)]

Crashing in |nsJavaXPTCStub::FinalizeJavaParams()| due to over-aggressive Release() of xpcom object.  Specifically, this line:

     } else if (!isXPTCStub) { // if is native XPCOM object
       xpcom_obj->Release();
     }

Comment 1

[jhp (no longer active)]

Attachment: patch

The |xpcom_obj->Release();| was a bad attempt (I think) at handling the refcnt of xpcom objects for 'inout' params.  If the xpcom object that is returned by the method call is different than the object that was originally passed in, then the original xpcom object needs to be released.  This patch does it better.

Comment 2

[jhp (no longer active)]

Comment on attachment 203537 [details] [diff] [review]
patch

Shaver, does my description above about handling 'inout' params seem legit?

Comment 3

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

Honestly, inout parameters always make me write down a bunch of use cases and test them out in my head.  Your reasoning sounds sane to me, as an optimization of always doing NS_IF_ADDREF(newval); NS_IF_RELEASE(oldval);, but it still feels weird for a callee to be dropping a reference on an argument.

I think you want a second opinion!

Comment 4

[jhp (no longer active)]

Checked in to trunk to fix crash for now.  I'll keep this open to get more input.

Comment 5

[jhp (no longer active)]

Darin, what do you think of this patch?  See comments #1 and #3.

Comment 6

[Benjamin Smedberg]

Fixed on MOZILLA_1_8_BRANCH and MOZILLA_1_8_0_BRANCH.