698075 - Potential memory leak in AsyncChannel::Echo(), ::Send()

Status: RESOLVED FIXED

(Core :: IPC, defect)

Description

[Niko Matsakis [:nmatsakis]]

These methods take ownership of the Message* parameter, but in the case of error they do not delete it.

Comment 1

[Niko Matsakis [:nmatsakis]]

Attachment: Convert affected functions to use nsAutoptr<> and thus guarantee msg is freed

Comment 2

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

Comment on attachment 570336 [details] [diff] [review]
Convert affected functions to use nsAutoptr<> and thus guarantee msg is freed

The same fix is needed in RPCChannel::Call.

Please add that, then post another version of the patch and re-request review from me.  This looks good otherwise.

Comment 3

[Niko Matsakis [:nmatsakis]]

Attachment: Add use of nsAutoPtr<> into RPCChannel::Call

Comment 4

[Mozilla RelEng Bot]

Try run for 2c4a388f862d is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=2c4a388f862d
Results (out of 247 total builds):
    exception: 1
    success: 226
    warnings: 16
    failure: 4
Builds available at http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/[e-mail]-2c4a388f862d

Comment 5

[Mozilla RelEng Bot]

Try run for 4d691c3173e6 is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=4d691c3173e6
Results (out of 247 total builds):
    exception: 1
    success: 225
    warnings: 16
    failure: 5
Builds available at http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/[e-mail]-4d691c3173e6

Comment 6

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

For the run in comment 4, the whole enchilada, here's the breakdown.

 - https://tbpl.mozilla.org/?tree=Try&rev=2c4a388f862d is the link you want
 - the "Linux debug" M (... 2 ...) "orange" doesn't look familiar.  I requested another run using the "+" button in the bottom-middle of TBPL.
 - the red "JP" tests, jetpack, are broken so often that no one pays attention to them.  Sad state of affairs.  No harm done by your patch patches.
 - the "Win debug" M(1 ...) "orange" doesn't look familiar either, so I requested another run.

If the linux Md2 (mochitest, debug, part 2) and windows Md1 (mochitest, debug, part 1) runs come back green, I would recommend landing.  Otherwise we probably have a bug to chase down.

Comment 7

[Niko Matsakis [:nmatsakis]]

Looks like both came back green.  I will therefore push the patch (once I figure out how).

Comment 8

[Ed Morley [:emorley]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/1a01206eba24

To save time for future patches (this one is fine for now), could you set your hgrc to include the author automatically & also add a commit message, along the lines of:
https://developer.mozilla.org/en/Mercurial_FAQ#How_can_I_generate_a_patch_for_somebody_else_to_check-in_for_me.3F

Thanks :-)

Comment 9

[Marco Bonardo [:mak]]

https://hg.mozilla.org/mozilla-central/rev/1a01206eba24