New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 615440 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: May 2016
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 1
Type: Bug

Blocking:
issue 614909



Sign in to add a comment

Simple Cache: read-only entries are corrupted

Project Member Reported by gavinp@chromium.org, May 27 2016

Issue description

The new simple cache seek-saving optimisation has a bug. Since we write the headers on every close of an entry, including the SHA256, but we only write the stream 0 crc record when necessary, if an entry is opened, and read without being written, then on close the header will be corrupted.

 

Comment 1 by gavinp@chromium.org, May 27 2016

Blocking: 614909

Comment 2 by gavinp@chromium.org, May 27 2016

Components: Internals>Network>Cache>Simple
Labels: ReleaseBlock-Stable Arch-All M-52
Project Member

Comment 3 by bugdroid1@chromium.org, May 27 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/6b66971b761a15a67982bef692619a96f306150d

commit 6b66971b761a15a67982bef692619a96f306150d
Author: gavinp <gavinp@chromium.org>
Date: Fri May 27 19:33:00 2016

Simple Cache: avoid extraneous stream 0 (http header) writes.

These writes were not only wasteful, but they could cause corruption
if the existing EOF record didn't have a key SHA256; the new key
SHA256 would overwrite the existing one corrupting the entry.

BUG= 615440 
R=juliatuttle@chromium.org,rdsmith@chromium.org

Review-Url: https://codereview.chromium.org/2015263002
Cr-Commit-Position: refs/heads/master@{#396533}

[modify] https://crrev.com/6b66971b761a15a67982bef692619a96f306150d/net/disk_cache/entry_unittest.cc
[modify] https://crrev.com/6b66971b761a15a67982bef692619a96f306150d/net/disk_cache/simple/simple_synchronous_entry.cc

Comment 4 by gavinp@chromium.org, May 27 2016

Labels: Merge-Request-52
Status: Fixed (was: Started)
I will let this bake on trunk, but this definitely needs a merge to the branch after it's validated. Setting merge requested.

Comment 5 by tin...@google.com, May 28 2016

Labels: -Merge-Request-52 Merge-Approved-52 Hotlist-Merge-Approved
Your change meets the bar and is auto-approved for M52 (branch: 2743)
Project Member

Comment 6 by bugdroid1@chromium.org, May 31 2016

Labels: -merge-approved-52 merge-merged-2743
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/53f45d9bb2f4c767fc1edbed08d31edecaeb4fff

commit 53f45d9bb2f4c767fc1edbed08d31edecaeb4fff
Author: Gavin Peters <gavinp@chromium.org>
Date: Tue May 31 14:46:03 2016

Simple Cache: avoid extraneous stream 0 (http header) writes.

These writes were not only wasteful, but they could cause corruption
if the existing EOF record didn't have a key SHA256; the new key
SHA256 would overwrite the existing one corrupting the entry.

BUG= 615440 
R=juliatuttle@chromium.org,rdsmith@chromium.org

Review-Url: https://codereview.chromium.org/2015263002
Cr-Commit-Position: refs/heads/master@{#396533}
(cherry picked from commit 6b66971b761a15a67982bef692619a96f306150d)

Review URL: https://codereview.chromium.org/2021303002 .

Cr-Commit-Position: refs/branch-heads/2743@{#138}
Cr-Branched-From: 2b3ae3b8090361f8af5a611712fc1a5ab2de53cb-refs/heads/master@{#394939}

[modify] https://crrev.com/53f45d9bb2f4c767fc1edbed08d31edecaeb4fff/net/disk_cache/entry_unittest.cc
[modify] https://crrev.com/53f45d9bb2f4c767fc1edbed08d31edecaeb4fff/net/disk_cache/simple/simple_synchronous_entry.cc

Sign in to add a comment