Closed Bug 1344171 Opened 7 years ago Closed 7 years ago

Improve connection management

Categories

(Core :: Networking: HTTP, enhancement)

enhancement
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla55
Tracking Status
firefox55 --- fixed

People

(Reporter: dragana, Assigned: dragana)

References

Details

(Whiteboard: [necko-active])

Attachments

(1 file, 4 obsolete files)

1) if a connection is read, i.e. socket is connected, and we can attach a pending transaction or a NullTransaction, for https connections, it can happen that the connection is put into idle queue even though it could take a NullTransaction to perform tls. This can happen when the transaction pending queue is not empty but all transaction in the queue are content-blocked.

2) If transaction takes a speculatively open halfOpenSocket, the halfOpenSocket will not backup socket as in other cases.

3) If a halfOpenSocket is opened for a transaction, but the transaction takes another connection, the halfOpenSocket cannot be claimed by another transaction.
Assignee: nobody → dd.mozilla
Status: NEW → ASSIGNED
Whiteboard: [necko-active]
Depends on: 1341572
Attached patch bug_1344171_v1.patch (obsolete) — Splinter Review
Attachment #8844421 - Flags: review?(mcmanus)
Comment on attachment 8844421 [details] [diff] [review]
bug_1344171_v1.patch

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

oddly, this seems simpler. thanks!
Attachment #8844421 - Flags: review?(mcmanus) → review+
Attached patch bug_1344171_v1.patch (obsolete) — Splinter Review
Rebased.
Attachment #8844421 - Attachment is obsolete: true
Attachment #8849068 - Flags: review+
Keywords: checkin-needed
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/1f0db7810a84
Improve connection management. r=mcmanus
Keywords: checkin-needed
Backed out for bustage (unused variable 'claimed'):

https://hg.mozilla.org/integration/mozilla-inbound/rev/0afa705631ce000b3eef48deef5d618b510a2fc0

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=8646978d35260c7a796f776d5d03b9d62828bbaf&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=retry&filter-resultStatus=usercancel&filter-resultStatus=runnable
Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=85117771&repo=mozilla-inbound

[task 2017-03-20T19:25:59.855081Z] 19:25:59     INFO -  In file included from /home/worker/workspace/build/src/obj-firefox/netwerk/protocol/http/Unified_cpp_protocol_http1.cpp:110:0:
[task 2017-03-20T19:25:59.855394Z] 19:25:59     INFO -  /home/worker/workspace/build/src/netwerk/protocol/http/nsHttpConnectionMgr.cpp: In member function 'nsresult mozilla::net::nsHttpConnectionMgr::CreateTransport(mozilla::net::nsHttpConnectionMgr::nsConnectionEntry*, mozilla::net::nsAHttpTransaction*, uint32_t, bool, bool, bool, mozilla::net::nsHttpConnectionMgr::PendingTransactionInfo*)':
[task 2017-03-20T19:25:59.856385Z] 19:25:59     INFO -  /home/worker/workspace/build/src/netwerk/protocol/http/nsHttpConnectionMgr.cpp:1826:14: error: unused variable 'claimed' [-Werror=unused-variable]
[task 2017-03-20T19:25:59.856581Z] 19:25:59     INFO -           bool claimed = sock->Claim();
[task 2017-03-20T19:25:59.856760Z] 19:25:59     INFO -                ^
[task 2017-03-20T19:25:59.856949Z] 19:25:59     INFO -  cc1plus: all warnings being treated as errors
[task 2017-03-20T19:25:59.857182Z] 19:25:59     INFO -  /home/worker/workspace/build/src/config/rules.mk:1016: recipe for target 'Unified_cpp_protocol_http1.o' failed
[task 2017-03-20T19:25:59.857359Z] 19:25:59     INFO -  gmake[5]: *** [Unified_cpp_protocol_http1.o] Error 1
Flags: needinfo?(dd.mozilla)
Attached patch bug_1344171_v1.patch (obsolete) — Splinter Review
Attachment #8849068 - Attachment is obsolete: true
Flags: needinfo?(dd.mozilla)
Attachment #8849623 - Flags: review+
Pushed by dd.mozilla@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/c040782ca691
Improve connection management. r=mcmanus
Attached patch bug_1344171_v1.patch (obsolete) — Splinter Review
Attachment #8849623 - Attachment is obsolete: true
Flags: needinfo?(dd.mozilla)
Attachment #8852081 - Flags: review+
Keywords: checkin-needed
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/d11fb28f9b9c
Improve connection management. r=mcmanus
Keywords: checkin-needed
Backed out in https://hg.mozilla.org/integration/mozilla-inbound/rev/552267771708 for pretty much the same Win7 test_altsvc.js crashes, e.g. https://treeherder.mozilla.org/logviewer.html#?job_id=87151902&repo=mozilla-inbound (the rest of the failures after test_altsvc.js are unrelated, just other bustage from another busted patch in the same push).
(In reply to Phil Ringnalda (:philor) from comment #13)
> Backed out in
> https://hg.mozilla.org/integration/mozilla-inbound/rev/552267771708 for
> pretty much the same Win7 test_altsvc.js crashes, e.g.
> https://treeherder.mozilla.org/logviewer.html#?job_id=87151902&repo=mozilla-
> inbound (the rest of the failures after test_altsvc.js are unrelated, just
> other bustage from another busted patch in the same push).

Thank you. So sorry this was my mistake, I am on IETF and a bit distracted. sorry!
Attachment #8852081 - Attachment is obsolete: true
Attachment #8853065 - Flags: review+
Keywords: checkin-needed
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/364acbf6a069
Improve connection management. r=mcmanus
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/364acbf6a069
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: