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

RESOLVED FIXED

Status

Core Graveyard
Java to XPCOM Bridge
RESOLVED FIXED
12 years ago
4 years ago

People

(Reporter: jhp (no longer active), Assigned: jhp (no longer active))

Tracking

({fixed1.8.0.1, fixed1.8.1})

Trunk
fixed1.8.0.1, fixed1.8.1

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

12 years ago
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();
     }
(Assignee)

Comment 1

12 years ago
Created attachment 203537 [details] [diff] [review]
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.
(Assignee)

Comment 2

12 years ago
Comment on attachment 203537 [details] [diff] [review]
patch

Shaver, does my description above about handling 'inout' params seem legit?
Attachment #203537 - Flags: review?(shaver)
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!
(Assignee)

Comment 4

12 years ago
Checked in to trunk to fix crash for now.  I'll keep this open to get more input.
(Assignee)

Updated

12 years ago
Attachment #203537 - Flags: review?(shaver)
(Assignee)

Comment 5

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

Comment 6

12 years ago
Fixed on MOZILLA_1_8_BRANCH and MOZILLA_1_8_0_BRANCH.
Keywords: fixed1.8.0.1, fixed1.8.1
(Assignee)

Updated

12 years ago
Status: NEW → RESOLVED
Last Resolved: 12 years ago
Resolution: --- → FIXED
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.