Eliminate extra refcount in nsHttpConnectionMgr:: nsHalfOpenSocket::OnOutputStreamReady()

RESOLVED INVALID

Status

()

Core
Networking: HTTP
P3
normal
RESOLVED INVALID
a year ago
a month ago

People

(Reporter: Igor, Assigned: valentin)

Tracking

Trunk
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [necko-backlog])

(Reporter)

Description

a year ago
User Agent: Mozilla/5.0 (Windows NT 6.3; WOW64; rv:36.0) Gecko/20100101 Firefox/36.0
Build ID: 20150305021524
(Reporter)

Comment 1

a year ago
Eliminate the  extra refcounting in

RefPtr<nsHttpTransaction> temp = mEnt->mPendingQ[*index].forget();

::: netwerk/protocol/http/nsHttpConnectionMgr.cpp
@@ +3367,4 @@
>          MOZ_ASSERT(!mSpeculative,
>                     "Speculative Half Open found mTransaction");
> +        RefPtr<nsHttpTransaction> temp = mEnt->mPendingQ[*index];
> +        mEnt->mPendingQ.RemoveElementAt(*index);

by doing:

RefPtr<nsHttpTransaction> temp = mEnt->mPendingQ[*index].forget();
(Reporter)

Updated

a year ago
OS: Unspecified → All
Hardware: Unspecified → All
The code doesn't look exactly like that anymore. If you suggest a change, I think it would make more sense to discuss on the contents of mozilla-central. Can you make it a full and proper patch?
Flags: needinfo?(palmieri.igor)
(Reporter)

Comment 3

a year ago
Hi,

This is a follow-up to bug 1296610, opened on request by the reviewer:

https://bugzilla.mozilla.org/show_bug.cgi?id=1296610

But it is the first time I fill a bug, so I am not sure I did it right. The code is slightly different now, but is expected to be like I quoted after the patch of bug 1296610.

I believe that the patch will only add the .forget() on the temp definition.

Please let me know if there is anything that must be fixed.
Flags: needinfo?(palmieri.igor)
(Assignee)

Comment 4

a year ago
Seems to like the change suggested in comment 0 would actually leave a null pointer in the queue.
Let's take another look at this after bug 1296610 lands.
Assignee: nobody → valentin.gosu
Whiteboard: [necko-backlog]
Bulk change to priority: https://bugzilla.mozilla.org/show_bug.cgi?id=1399258
Priority: -- → P1
Bulk change to priority: https://bugzilla.mozilla.org/show_bug.cgi?id=1399258
Priority: P1 → P3
(Assignee)

Comment 7

a month ago
I think this bug no longer applies. Please reopen if that's not the case.
Status: UNCONFIRMED → RESOLVED
Last Resolved: a month ago
Resolution: --- → INVALID
You need to log in before you can comment on or make changes to this bug.