This is a spin-off from bug #548406, comment #282. If the cache receives two read-write ("normal") requests followed by one force-write request (all requests for the same resource) we hit an assertion when trying to insert a new entry for the third request into the list of active entries.
This problem comes from the fact that we call DoomEntry_Internal(|existing entry|) in ActivateEntry for the third (force-write) request. This in turn calls ProcessPendingRequests which ends up calling ActivateEntry again (a recursion, in other words) which results in a new entry added to mActiveEntries. Control eventually returns to the point where we called DoomEntry_Internal, and here we clear our |entry| and in the next step we create another new entry and try to insert it into mActiveEntries. Boom... IMO, a fix for this could be related to a fix for bug #548406, especially if the latter includes keeping the existing entry somehow.
Followup from bug #548406, comment #282. > But I don't think the result must be deterministic in this case. The cache > doesn't guarantee that you can always read back what you've written. And this > is exactly the case. HTTP request #3 wants to bypass the cache (and the caller > probably knows why he asked that) so a new content is expected and the cache > should be updated. And in our case this means that HTTP request #2 won't use > cache (which may be obsolete). This seems to me reasonable. You're probably right. But there is still something bothering me about this which I cannot quite put my finger on... If we allow HTTP request #3 to cancel cache-access for HTTP request #2 we swap the order of requests #2 and #3 as seen by the server. Problem? Depending on timing between requests and speed of cache-device, request #2 will either be read from cache or fetched from the server. Problem? > Also please note > that your testcase is artificial test that IMO doesn't reflect real scenarios. Sure, but doesn't that apply to many unittests? :) We establish some desired behavior for our software and create unittests to ensure isolated aspects of this behavior. (The interesting bit in this particular case, IMO, is that desired behavior is a little unclear.) > Ah, OK. Anyway IMO the issue isn't limited to unbound entries. The test would > assert even if the entry would be already bound. True. > > A thought-experiment : Three XMLHttpRequests access a resource like the test > > does (first two requests are normal, third has a bypass-cache flag). Which > > result-value would we expect for the second XMLHttpRequest? > > I would expect the correct response from the server (i.e. options 1). If the > server's response differs from the cache content then we return right response. > If the response is the same then we just sent one inefficient request to the > server. Mmmmm... that corresponds to changing the second channel-load in the xpcshell-test to expect "2" and not "1" . However, that would not be true if the third load was not present or was delayed. Problem (in practice)?
Using the updated unit-test from bug #596443, comment #36 I believe this bug is fixed by the patch in bug #596443, comment #41.
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.