bugzilla.mozilla.org will be intermittently unavailable on Saturday, March 24th, from 16:00 until 20:00 UTC.
Scenario: - http channel open a cache entry - gets a new one (in an EMPTY state) - makes a network request - headers come back to it - fills metadata on the entry - calls MetaDataReady() on the entry (entry shifts to READY state) - http channel abandons the entry before it opens the output stream on the entry (OnHandleClosed is called on the entry) ; this may happen when the channel is canceled from OnStartRequest of its listener or when OnStartRequest of the listener throws (InstallCacheListener that opens the output stream happens after that) - later, another channel opens the entry - gets a READY entry with metadata only filled - checks on the entry size - it returns IN_PROGRESS since mHasData on the entry is left false (output stream has never been actually open on it) - http channel decides to wait for a complete entry (e.g. because it must be validated first) => callback never happens Solution is to pretend the output stream has been open after the writer threw away the entry. The data length is actually 0, but it's a valid data length - next consumer will do a Range: 0- request that should by the spec work. Other option is to revert to EMPTY state, but it would throw the metadata away while it's definitely OK to have a no-content entry.
Created attachment 8448964 [details] [diff] [review] v1 https://tbpl.mozilla.org/?tree=Try&rev=412d7c58edcb
Attachment #8448964 - Flags: review?(michal.novotny)
Any idea how often this bug happens in practice? Given how simple the fix it I'm tempted to uplift it as far as we can, since it's bad when it happens.
And by bad I mean that the load will keep failing unless you do a shift-reload, right?
(In reply to Jason Duell (:jduell) from comment #3) > And by bad I mean that the load will keep failing unless you do a > shift-reload, right? exactly. (In reply to Jason Duell (:jduell) from comment #2) > Any idea how often this bug happens in practice? I think I have hit it few times, but rarely. > Given how simple the fix it > I'm tempted to uplift it as far as we can, since it's bad when it happens. It looks simple, but this flag is pretty sensitive and I've added it with that in mind... When something here goes wrong, we may be pretty fucked. So I'd rather wait few days before any rush uplifts.
Attachment #8448964 - Flags: review?(michal.novotny) → review+
backed out as https://hg.mozilla.org/integration/mozilla-inbound/rev/b60afce726d1 for: TEST-UNEXPECTED-FAIL | /builds/slave/talos-slave/test/build/tests/xpcshell/tests/netwerk/test/unit/test_cache2-26-no-outputstream-open.js | Test timed out TEST-UNEXPECTED-FAIL | /builds/slave/talos-slave/test/build/tests/xpcshell/tests/netwerk/test/unit/test_cache2-26-no-outputstream-open.js | test failed (with xpcshell return code: -1), see following log: For reference, that this went though try, undetected: https://tbpl.mozilla.org/?tree=Try&rev=412d7c58edcb Apparently GC is not reliable here.
The reason why try has passed is that this new test was mistakenly disabled on that push. I'll repush with that change reverted.
Created attachment 8450131 [details] [diff] [review] v1.1 - test disabled on And. https://tbpl.mozilla.org/?tree=Try&rev=a9bf708d80c4 - last try
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla33
You need to log in before you can comment on or make changes to this bug.