IndexedDB leaks attached Blobs under read-modify-write access pattern |
||||||||||
Issue descriptionBackground: Given an IndexedDB record (object store entry) with an attached Blob, when a Web page reads the record, modifies the in-memory copy, and overwrites the record with the modified copy, the new record version has a copy of the attached Blob. Bug: If the Web page holds on to the Blob attached to the old version of the record while overwriting, the file backing the old Blob is leaked. The leaked file remains on disk until the whole IndexedDB is removed, or until the whole origin's data is cleared. This can cause unbounded database growth in a workload that uses Blobs and indefinitely overwrites the same records, while holding on to the Blobs. JS garbage collection makes it very easy for a Web page to hold onto a Blob, even if it doesn't intend to. An example workload is a virtual disk implementation that stores blocks in Blobs keyed by block ID would. Assigning to myself, as I will add reproduction steps and preliminary results.
,
Aug 19 2017
The test passes (bug is not present) on M60 (60.0.3112.101) on Mac. Using blobs: true Database size: 256 Round count: 70 Quota -- Used: 7.02 MB Total: 303.47 MB Creating database Starting round 0 / 70 Quota -- Used: 71.05 MB Total: 270.25 MB Starting round 10 / 70 Quota -- Used: 270.50 MB Total: 270.25 MB Starting round 20 / 70 Quota -- Used: 270.50 MB Total: 270.25 MB Starting round 30 / 70 Quota -- Used: 270.50 MB Total: 270.25 MB Starting round 40 / 70 Quota -- Used: 270.50 MB Total: 270.25 MB Starting round 50 / 70 Quota -- Used: 270.50 MB Total: 270.25 MB Starting round 60 / 70 Quota -- Used: 270.50 MB Total: 270.25 MB Done Quota -- Used: 7.02 MB Total: 303.47 MB
,
Aug 19 2017
The test fails (bug is present) on M61 (61.0.3163.49) on Mac. Using blobs: true Database size: 256 Quota -- Used: 0.00 MB Total: 25818.71 MB Creating database Starting round 0 / 70 Quota -- Used: 64.02 MB Total: 25763.71 MB Starting round 10 / 70 Quota -- Used: 709.80 MB Total: 25818.78 MB Starting round 20 / 70 Quota -- Used: 1354.36 MB Total: 25818.20 MB Starting round 30 / 70 Quota -- Used: 1994.99 MB Total: 25813.45 MB Starting round 40 / 70 Quota -- Used: 2639.55 MB Total: 25813.29 MB Starting round 50 / 70 Quota -- Used: 3267.87 MB Total: 25800.75 MB Starting round 60 / 70 Quota -- Used: 3912.07 MB Total: 25788.42 MB Done Quota -- Used: 0.00 MB Total: 25818.71 MB
,
Aug 19 2017
The test passes (bug is not present) on M62 (62.0.3189.0) on Mac. Using blobs: true Database size: 256 Round count: 70 Quota -- Used: 0.00 MB Total: 328.97 MB Creating database Starting round 0 / 70 Quota -- Used: 64.02 MB Total: 320.83 MB Starting round 10 / 70 Quota -- Used: 320.83 MB Total: 320.83 MB Starting round 20 / 70 Quota -- Used: 320.83 MB Total: 320.83 MB Starting round 30 / 70 Quota -- Used: 320.83 MB Total: 320.83 MB Starting round 40 / 70 Quota -- Used: 320.83 MB Total: 320.83 MB Starting round 50 / 70 Quota -- Used: 320.83 MB Total: 320.83 MB Starting round 60 / 70 Quota -- Used: 320.83 MB Total: 320.83 MB Done Quota -- Used: 0.00 MB Total: 328.97 MB
,
Aug 19 2017
Please help! I'd really like to have this tested against Stable / Beta / Dev / Canary on Windows / Mac / Linux. After that, please reassign to me, and I'll ask for some bisections. Thank you very much!
,
Aug 19 2017
pbommana@: Please help. I looked around go/chrome-te, and I couldn't figure out the right way to ask for TE help on a bug. Can you please take a look at Comment 2 and Comment 5, and route the bug to the right person? Also, if you point me to the instructions for asking for help, I'll share them (at the very least) with my team, so you'll have fewer people asking dumb questions :D Thank you very much for your help!
,
Aug 21 2017
Tested the issue on windows 10 ,ubuntu 14.04 and Mac OS 10.12.6 using chrome M60 #60.0.3112.101 , M61 #61.0.3163.49 and M62 #62.0.3192.0 and followed the steps mentioned in comment #1. Observed that all the chrome channels resulted in failure results , as the results are above 2GB . Attached screenshots of the results of all the channels. Thanks!
,
Aug 22 2017
hdodda@: Can you please run the tests again? Sorry, my instructions above were unclear and incomplete. You need to look at the "Used" number, not at the "Total" number. If that number goes above 2GB, the test failed. In all the screenshots you have posted, the Used quota is below 1GB, so the test passed. I apologize for being unclear earlier, and thank you very much for your help!
,
Aug 22 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/e0433a1fcc97551dfb9fc1774411b9d2db273b75 commit e0433a1fcc97551dfb9fc1774411b9d2db273b75 Author: Daniel Murphy <dmurph@chromium.org> Date: Tue Aug 22 23:01:42 2017 [BlobStorage] Adding DVLOG for debugging help R: pwnall Bug: 757142 , 756754 Change-Id: Id055826191299965eddf8640eb09c640ad3f58c3 Reviewed-on: https://chromium-review.googlesource.com/627019 Commit-Queue: Daniel Murphy <dmurph@chromium.org> Reviewed-by: Victor Costan <pwnall@chromium.org> Cr-Commit-Position: refs/heads/master@{#496487} [modify] https://crrev.com/e0433a1fcc97551dfb9fc1774411b9d2db273b75/content/browser/blob_storage/blob_dispatcher_host.cc [modify] https://crrev.com/e0433a1fcc97551dfb9fc1774411b9d2db273b75/storage/browser/blob/blob_storage_context.cc [modify] https://crrev.com/e0433a1fcc97551dfb9fc1774411b9d2db273b75/storage/browser/blob/scoped_file.cc [modify] https://crrev.com/e0433a1fcc97551dfb9fc1774411b9d2db273b75/storage/browser/blob/scoped_file.h [modify] https://crrev.com/e0433a1fcc97551dfb9fc1774411b9d2db273b75/storage/browser/blob/shareable_file_reference.cc
,
Aug 23 2017
Tested the issue again on windows 10 ,ubuntu 14.04 and Mac OS 10.12.6 using chrome M60 #60.0.3112.101 , M61 #61.0.3163.49 and M62 #62.0.3193.0 and as per comment #8, all the test results passed as the used quota is below 1 GB as attached screenshots in comment #7. @pwnall-- Please let us know if this is the expected behavior , so that we can bisect the issue. Thanks!
,
Aug 23 2017
hdodda@: Thank you very much for testing! The results are a surprise, as the test fails for me on M61 on Mac. Removing the bisect request until I can figure out what's going on. I'm suspecting that the test isn't reliable. (could be hitting a race condition somewhere)
,
Aug 23 2017
,
Oct 3 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/ac35e5368b36263695abddad60257672791f184f commit ac35e5368b36263695abddad60257672791f184f Author: Daniel Murphy <dmurph@chromium.org> Date: Tue Oct 03 01:28:12 2017 [IndexedDB] Fixed blob leaking and delayed deletion. * Ensures blob cleanup is run on database close, instead of leaving undeleted old blobs on disk. * Enforce a max time for the blob cleanup timer, as repeated blob operations made blobs never actually clean up. * Notify the quota manager when blobs are cleaned up to update quota. Bug: 756754 , 757142 Change-Id: I62fd79f25aa80e08df4aa631ae40a6aebd2726e0 Reviewed-on: https://chromium-review.googlesource.com/658153 Commit-Queue: Daniel Murphy <dmurph@chromium.org> Reviewed-by: Victor Costan <pwnall@chromium.org> Reviewed-by: Joshua Bell <jsbell@chromium.org> Cr-Commit-Position: refs/heads/master@{#505908} [modify] https://crrev.com/ac35e5368b36263695abddad60257672791f184f/content/browser/indexed_db/indexed_db_backing_store.cc [modify] https://crrev.com/ac35e5368b36263695abddad60257672791f184f/content/browser/indexed_db/indexed_db_backing_store.h [modify] https://crrev.com/ac35e5368b36263695abddad60257672791f184f/content/browser/indexed_db/indexed_db_backing_store_unittest.cc [modify] https://crrev.com/ac35e5368b36263695abddad60257672791f184f/content/browser/indexed_db/indexed_db_browsertest.cc [modify] https://crrev.com/ac35e5368b36263695abddad60257672791f184f/content/browser/indexed_db/indexed_db_context_impl.cc [modify] https://crrev.com/ac35e5368b36263695abddad60257672791f184f/content/browser/indexed_db/indexed_db_context_impl.h [modify] https://crrev.com/ac35e5368b36263695abddad60257672791f184f/content/browser/indexed_db/indexed_db_factory.h [modify] https://crrev.com/ac35e5368b36263695abddad60257672791f184f/content/browser/indexed_db/indexed_db_factory_impl.cc [modify] https://crrev.com/ac35e5368b36263695abddad60257672791f184f/content/browser/indexed_db/indexed_db_factory_impl.h [modify] https://crrev.com/ac35e5368b36263695abddad60257672791f184f/content/browser/indexed_db/mock_indexed_db_factory.h
,
Oct 3 2017
,
Oct 10 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/18580919bc4e8a90a366459cad1f7b51771d674a commit 18580919bc4e8a90a366459cad1f7b51771d674a Author: Daniel Murphy <dmurph@chromium.org> Date: Tue Oct 10 18:58:12 2017 [IndexedDB] Fixed blob leaking and delayed deletion. * Ensures blob cleanup is run on database close, instead of leaving undeleted old blobs on disk. * Enforce a max time for the blob cleanup timer, as repeated blob operations made blobs never actually clean up. * Notify the quota manager when blobs are cleaned up to update quota. TBR=dmurph@chromium.org (cherry picked from commit ac35e5368b36263695abddad60257672791f184f) Bug: 756754 , 757142 Change-Id: I62fd79f25aa80e08df4aa631ae40a6aebd2726e0 Reviewed-on: https://chromium-review.googlesource.com/658153 Commit-Queue: Daniel Murphy <dmurph@chromium.org> Reviewed-by: Victor Costan <pwnall@chromium.org> Reviewed-by: Joshua Bell <jsbell@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#505908} Reviewed-on: https://chromium-review.googlesource.com/710158 Reviewed-by: Daniel Murphy <dmurph@chromium.org> Cr-Commit-Position: refs/branch-heads/3202@{#638} Cr-Branched-From: fa6a5d87adff761bc16afc5498c3f5944c1daa68-refs/heads/master@{#499098} [modify] https://crrev.com/18580919bc4e8a90a366459cad1f7b51771d674a/content/browser/indexed_db/indexed_db_backing_store.cc [modify] https://crrev.com/18580919bc4e8a90a366459cad1f7b51771d674a/content/browser/indexed_db/indexed_db_backing_store.h [modify] https://crrev.com/18580919bc4e8a90a366459cad1f7b51771d674a/content/browser/indexed_db/indexed_db_backing_store_unittest.cc [modify] https://crrev.com/18580919bc4e8a90a366459cad1f7b51771d674a/content/browser/indexed_db/indexed_db_browsertest.cc [modify] https://crrev.com/18580919bc4e8a90a366459cad1f7b51771d674a/content/browser/indexed_db/indexed_db_context_impl.cc [modify] https://crrev.com/18580919bc4e8a90a366459cad1f7b51771d674a/content/browser/indexed_db/indexed_db_context_impl.h [modify] https://crrev.com/18580919bc4e8a90a366459cad1f7b51771d674a/content/browser/indexed_db/indexed_db_factory.h [modify] https://crrev.com/18580919bc4e8a90a366459cad1f7b51771d674a/content/browser/indexed_db/indexed_db_factory_impl.cc [modify] https://crrev.com/18580919bc4e8a90a366459cad1f7b51771d674a/content/browser/indexed_db/indexed_db_factory_impl.h [modify] https://crrev.com/18580919bc4e8a90a366459cad1f7b51771d674a/content/browser/indexed_db/mock_indexed_db_factory.h |
||||||||||
►
Sign in to add a comment |
||||||||||
Comment 1 by pwnall@chromium.org
, Aug 19 2017Labels: OS-All
5.3 KB
5.3 KB View Download