1411316 - api for cancelling existing transactions

Status: RESOLVED FIXED

(Core :: Networking, enhancement, P2)

Description

[Patrick McManus [:mcmanus]]

there is not currently an API for closing all existing connections abruptly without shutting down (you can do so gracefully). Apparently this would help with NSS shutdown, so here we go.

I'm not going to write a test until we have some feedback that it is solving the problem david is aiming at..

Comment 1

[Patrick McManus [:mcmanus]]

Attachment: Bug 1411316 - necko api for cancelling all transactions

Review commit: https://reviewboard.mozilla.org/r/192546/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/192546/

Comment 2

[Dana Keeler (she/her) (use needinfo) [:keeler] (on leave)]

Yep - this is exactly what I was looking for - Thanks! Sorry I took a while to get to this...

Comment 3

[Dana Keeler (she/her) (use needinfo) [:keeler] (on leave)]

Whoops - forgot need info. See comment 2.

Comment 4

[Patrick McManus [:mcmanus]]

nick, do you think you could write a test for this and review the code? (I can review the test).

Comment 5

[J.C. Jones [:jcj] (he/him)]

Nick,

Since this could have ... interesting ... implications, we'd like to get this in for as much of this new Nightly cycle as we can for maximum testing time.

Comment 6

[u408661]

I'm pretty swamped as it is. I'm happy to review code, but it would take me longer to come up to speed enough to write a test when you're already familiar with this.

Comment 7

[Kershaw Chang [:kershaw]]

(In reply to Nicholas Hurley [:nwgh][:hurley] (also [e-mail]) from comment #6)
> I'm pretty swamped as it is. I'm happy to review code, but it would take me
> longer to come up to speed enough to write a test when you're already
> familiar with this.

I'll try to write a test for this.

Comment 8

[Kershaw Chang [:kershaw]]

Attachment: Test case

I've created a test for this. Basically, the purpose of this test is checking whether all active connections are canceled or not. Please see the front of the test file for more details.

Not sure if this test is enough. Patrick, could you take a look at this patch?
Thanks.

Comment 9

[Patrick McManus [:mcmanus]]

Comment on attachment 8935675 [details] [diff] [review]
Test case

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

thanks

Comment 10

[Dragana Damjanovic [:dragana]]

Comment on attachment 8921526 [details]
Bug 1411316 - necko api for cancelling all transactions

https://reviewboard.mozilla.org/r/192546/#review212864

::: netwerk/protocol/http/nsHttpConnectionMgr.cpp
(Diff revision 1)
>          }
>  
> -        // If all idle connections are removed we can stop pruning dead
> -        // connections.
> -        ConditionallyStopPruneDeadConnectionsTimer();
> -

You can leave this here. It is ok to cancel the timer if we close all connections. The function check if there are active or idle connections.

Comment 11

[Patrick McManus [:mcmanus]]

Comment on attachment 8921526 [details]
Bug 1411316 - necko api for cancelling all transactions

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/192546/diff/1-2/

Comment 12

[Patrick McManus [:mcmanus]]

Comment on attachment 8921526 [details]
Bug 1411316 - necko api for cancelling all transactions

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/192546/diff/2-3/

Comment 13

[Patrick McManus [:mcmanus]]

Comment on attachment 8921526 [details]
Bug 1411316 - necko api for cancelling all transactions

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/192546/diff/3-4/

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/46b0f7f066e3
necko api for cancelling all transactions r=dragana

Comment 15

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/46b0f7f066e3