Last Comment Bug 690425 - HttpChannelChild incorrectly attempts to delete actor when kept alive connection dies
: HttpChannelChild incorrectly attempts to delete actor when kept alive connect...
Status: RESOLVED FIXED
[mentor=jdm] [lang=c++]
:
Product: Core
Classification: Components
Component: Networking: HTTP (show other bugs)
: Trunk
: x86 Mac OS X
-- normal (vote)
: mozilla13
Assigned To: Justin Lebar (not reading bugmail)
:
: Patrick McManus [:mcmanus]
Mentors:
Depends on:
Blocks: 588256
  Show dependency treegraph
 
Reported: 2011-09-29 10:49 PDT by Josh Matthews [:jdm]
Modified: 2012-02-09 10:25 PST (History)
4 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Stack trace demonstrating the problem (2.26 KB, text/plain)
2011-09-29 10:49 PDT, Josh Matthews [:jdm]
no flags Details
Patch v1 (1.22 KB, patch)
2012-02-08 14:21 PST, Justin Lebar (not reading bugmail)
josh: review+
jduell.mcbugs: review+
Details | Diff | Splinter Review

Description User image Josh Matthews [:jdm] 2011-09-29 10:49:33 PDT
Created attachment 563460 [details]
Stack trace demonstrating the problem

Easy way to reproduce:
1. build fennec
2. visit http://ted.mielczarek.org/mozilla/crashme.html, install the addon linked there
3. restart once the addon has installed
4. observe crash in content process
Comment 1 User image Josh Matthews [:jdm] 2011-09-29 10:54:58 PDT
Darn, I forgot to keep in mind that actors can die inadvertently when reviewing the changes in bug 588256. We should get rid of the mIPCOpen assertion in HttpChannelChild::Release and add mIPCOpen as a condition for the whole if branch instead.
Comment 3 User image Honza Bambas (:mayhemer) 2011-12-14 07:48:20 PST
Hmm, from the first look it seems that the parent is not giving the child channel information it is gone, right?  I though this never could happen.  (But maybe I'm out now).
Comment 4 User image Josh Matthews [:jdm] 2011-12-14 08:32:49 PST
The problem is that if a channel is kept alive in OnStopRequest, we just assume that we should send an IPDL message to delete the actor in Release. However, if we're releasing because the IPDL actor has been destroyed, we should not do this.
Comment 5 User image Justin Lebar (not reading bugmail) 2012-02-08 12:10:11 PST
Should we delete this if |mKeptAlive && mRefCnt == 1 && !mIPCOpen|, or should we just return mRefCnt?
Comment 6 User image Josh Matthews [:jdm] 2012-02-08 12:15:01 PST
If the refcount is 1 and mIPCOpen is false, that means that somebody is holding a reference to the channel. Deleting at this point would be bad.
Comment 7 User image Justin Lebar (not reading bugmail) 2012-02-08 14:21:21 PST
Created attachment 595534 [details] [diff] [review]
Patch v1
Comment 8 User image Jason Duell [:jduell] (needinfo me) 2012-02-08 17:28:22 PST
Comment on attachment 595534 [details] [diff] [review]
Patch v1

Review of attachment 595534 [details] [diff] [review]:
-----------------------------------------------------------------

Yup, looks good.  Thanks!
Comment 9 User image Jason Duell [:jduell] (needinfo me) 2012-02-08 21:23:13 PST
Landed: cd44c3d677ef
Comment 10 User image Ed Morley [:emorley] 2012-02-09 10:25:14 PST
https://hg.mozilla.org/mozilla-central/rev/cd44c3d677ef

Note You need to log in before you can comment on or make changes to this bug.