HTTP cache v2: provide a new entry to consumers waiting for onCacheEntryAvailable in case of doom

RESOLVED WONTFIX

Status

()

Core
Networking: Cache
--
major
RESOLVED WONTFIX
4 years ago
a year ago

People

(Reporter: mayhemer, Unassigned)

Tracking

(Blocks: 1 bug)

Trunk
x86_64
Windows 7
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [necko-backlog])

Attachments

(2 attachments)

(Reporter)

Description

4 years ago
Created attachment 813772 [details]
nspr.log.zip [entry=94bf790]

This comes much more obvious with bug 922741 and bug 922671 in.

Scenario:
- channel1 opens a new entry
- channel2 opens it too
- channel2 gets onCheck after the channel1 calls metaDataReady()
- channel2 want the entry complete (response not resumable)
- channel1 doomes the entry
- channel2 gets onAvail(NOT_FOUND)

The log shows a failed assertion hit during this scenario that uncovers this issue.  The fix there is just to drop the flag.


I think we can freely provide the channel1 with a new entry (recreated one).
(Reporter)

Comment 1

4 years ago
[ No longer blocks enabling cache2, expected to be fixed after cache2 is on ]
Blocks: 941841
(Reporter)

Updated

4 years ago
No longer blocks: 913806
(Reporter)

Updated

4 years ago
Blocks: 913806
(Reporter)

Updated

4 years ago
No longer blocks: 913806
(Reporter)

Updated

4 years ago
Blocks: 913806
(Reporter)

Updated

4 years ago
No longer blocks: 913806
(Reporter)

Comment 2

4 years ago
This shows up important.
Blocks: 913806
No longer blocks: 941841
Severity: normal → major
(Reporter)

Updated

4 years ago
Assignee: nobody → honzab.moz
Status: NEW → ASSIGNED
(Reporter)

Comment 3

4 years ago
Created attachment 8358724 [details] [diff] [review]
v1 (not widely tested | unstable)

callbacks are now transferred when:
- Recreate()'ed (already implemeted, but now a bit improved)
- AsyncDoom()'ed
- OPEN_TRANCATE'ed

All should be clear from the patch.  The biggest difference is that callbacks should never be invoked on a doomed entry unless something during reopen fails (what shouldn't).

https://tbpl.mozilla.org/?tree=Try&rev=b732d195e645
(Reporter)

Comment 4

4 years ago
- the patch may potentially leak during shutdown, locally fixed
(Reporter)

Updated

4 years ago
Attachment #8358724 - Attachment description: v1 (not widely tested) → v1 (not widely tested | unstable)
(Reporter)

Comment 5

4 years ago
The patch is unstable and not critically needed for the first deployment.  Will return to this after enable.
Blocks: 941841
No longer blocks: 913806
Whiteboard: [necko-backlog]
(Reporter)

Comment 6

a year ago
(In reply to Honza Bambas (:mayhemer) from comment #2)
> This shows up important.

Unfortunately I didn't bother to write why... Anyway, the patch is rather complicated and I think this scenario is probably pretty rare.  Not worth the effort.
Assignee: honzab.moz → nobody
Status: ASSIGNED → RESOLVED
Last Resolved: a year ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.