300010 - Canceling a HTTP request while it is reading a partial cache entry does not cancel the corresponding transaction

Status: RESOLVED FIXED

(Core :: Networking: HTTP, defect, P1)

Description

[Darin Fisher]

Canceling a HTTP request while it is reading a partial cache entry does not
cancel the corresponding transaction.

When a HTTP range request is used to complete a partial cache entry, we first
issue a range request and if we get back a 206, then we read the cache and send
the partial cache data to our listener.  If the listener cancels the channel by
returning an error code (e.g., NS_BINDING_ABORTED) from their OnDataAvailable,
then the network transaction will never be closed.  It will just sit around,
consuming a persistent connection.  If this process is repeated, then subsequent
requests to the same host may get stuck waiting for an available persistent
connection.

Comment 1

[Darin Fisher]

Actually, this bug appears to affect the 1.7 branch only.  The transaction is
somehow getting closed properly on the trunk.

Comment 2

[Darin Fisher]

I take that back.  This can be reproduced on the trunk as well.

Comment 3

[Darin Fisher]

Attachment: v1 patch

Long term, I'd much rather make it so that errors returned from the chanenl's
nsIStreamListener simply ends up calling Cancel, but that turned out to be a
more involved change.  I decided to stick with something safe.	Anyways, byte
range requests used to complete partial cache entries is the only time that we
would have two requests active at a time (one for the cache and one for the
transaction).

Comment 4

[Christian :Biesinger (don't email me, ping me on IRC)]

Comment on attachment 188717 [details] [diff] [review]
v1 patch

can CancelTransaction lead to OnStopRequest being called again?

Comment 5

[Darin Fisher]

It will only get called once.  You can cancel a transaction as many times as you
like.  Moreover, the code is designed to handle OnStopRequest being called by
both mCachePump and mTransactionPump.

Comment 6

[Christian :Biesinger (don't email me, ping me on IRC)]

Comment on attachment 188717 [details] [diff] [review]
v1 patch

ok, cool

Comment 7

[Darin Fisher]

fixed-on-trunk