Auto-wrapped blobs don't get ack'd back to the browser process |
||||||||||
Issue descriptionThe auto-wrapping of blobs, on the unwrapping side, doesn't 'ack' the blob back to the browser process, so we end up with a dangling reference. This function needs to be called: https://cs.chromium.org/chromium/src/content/browser/indexed_db/database_impl.h?dr=CSs&l=121 with the wrapping blob uuid after it is finished unwrapping. Unfortunately the code right now only does that after unwrapping, where the 'wrapping' blob has been removed from the value.
,
Oct 24 2017
patch with more debugging help: https://chromium-review.googlesource.com/#/c/chromium/src/+/734642
,
Nov 4 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/b7b648d6180c470eeec2f94024ee1413421713c1 commit b7b648d6180c470eeec2f94024ee1413421713c1 Author: Daniel Murphy <dmurph@chromium.org> Date: Sat Nov 04 02:01:03 2017 [IndexedDB] Fixing blob leak when deserializing wrapped IDB large values * Fixed blob leak in IDBRequest for wrapped blobs, as unwrapping removes a yet-to-be-acked blob handle. * Fixed blob leak in IDBRequestLoader, which was reusing FileReaderLoader and thus leaking blob urls. Bug: 777665 , 777662 Change-Id: Icd3f7e8c9b30e39677ea024c206a9b61bf778fa8 Reviewed-on: https://chromium-review.googlesource.com/754020 Commit-Queue: Daniel Murphy <dmurph@chromium.org> Reviewed-by: Victor Costan <pwnall@chromium.org> Cr-Commit-Position: refs/heads/master@{#514011} [modify] https://crrev.com/b7b648d6180c470eeec2f94024ee1413421713c1/content/browser/indexed_db/indexed_db_backing_store.cc [modify] https://crrev.com/b7b648d6180c470eeec2f94024ee1413421713c1/third_party/WebKit/Source/modules/indexeddb/IDBRequest.cpp [modify] https://crrev.com/b7b648d6180c470eeec2f94024ee1413421713c1/third_party/WebKit/Source/modules/indexeddb/IDBRequestLoader.cpp [modify] https://crrev.com/b7b648d6180c470eeec2f94024ee1413421713c1/third_party/WebKit/Source/modules/indexeddb/IDBRequestTest.cpp [modify] https://crrev.com/b7b648d6180c470eeec2f94024ee1413421713c1/third_party/WebKit/Source/modules/indexeddb/IDBTransactionTest.cpp
,
Nov 6 2017
,
Nov 6 2017
Requesting merge to 63 - this fixes an issue where some large IDB values are 'leaky' every time they are read, which could result in unbounded memory usage if the site keeps reading the same value. It's a small change with tests.
,
Nov 6 2017
The bug is marked as P3 or Feature. It should not be merged as M63 is in beta. Please contact the approriate milestone owner if you have questions. Owners: cmasso@(Android), cmasso@(iOS), gkihumba@(ChromeOS), govind@(Desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Nov 6 2017
whoops, this is definitely a p2
,
Nov 6 2017
This bug requires manual review: M63 has already been promoted to the beta branch, so this requires manual review Please contact the milestone owner if you have questions. Owners: cmasso@(Android), cmasso@(iOS), gkihumba@(ChromeOS), govind@(Desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Nov 6 2017
Please add appropriate OSs.
,
Nov 6 2017
,
Nov 6 2017
I think this change is critical to get into m63 due to it's memory-leak nature. I don't think it should be an emergency re-role of m63, but I am really uncomfortable with this bug existing for so long in stable (these types of blobs were created & saved in m60 and m61).
,
Nov 6 2017
Approving merge to M63 branch 3239 based on comments #6 and #12. Please merge ASAP so we can take it in for this week M63 beta release. Thank you.
,
Nov 6 2017
,
Nov 6 2017
,
Nov 6 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/e061f37ea9677b36201b5cb8e30c01edbd748786 commit e061f37ea9677b36201b5cb8e30c01edbd748786 Author: Daniel Murphy <dmurph@chromium.org> Date: Mon Nov 06 21:35:54 2017 [IndexedDB] Fixing blob leak when deserializing wrapped IDB large values * Fixed blob leak in IDBRequest for wrapped blobs, as unwrapping removes a yet-to-be-acked blob handle. * Fixed blob leak in IDBRequestLoader, which was reusing FileReaderLoader and thus leaking blob urls. Bug: 777665 , 777662 Change-Id: Icd3f7e8c9b30e39677ea024c206a9b61bf778fa8 Reviewed-on: https://chromium-review.googlesource.com/754020 Commit-Queue: Daniel Murphy <dmurph@chromium.org> Reviewed-by: Victor Costan <pwnall@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#514011}(cherry picked from commit b7b648d6180c470eeec2f94024ee1413421713c1) Reviewed-on: https://chromium-review.googlesource.com/755822 Reviewed-by: Daniel Murphy <dmurph@chromium.org> Cr-Commit-Position: refs/branch-heads/3239@{#397} Cr-Branched-From: adb61db19020ed8ecee5e91b1a0ea4c924ae2988-refs/heads/master@{#508578} [modify] https://crrev.com/e061f37ea9677b36201b5cb8e30c01edbd748786/content/browser/indexed_db/indexed_db_backing_store.cc [modify] https://crrev.com/e061f37ea9677b36201b5cb8e30c01edbd748786/third_party/WebKit/Source/modules/indexeddb/IDBRequest.cpp [modify] https://crrev.com/e061f37ea9677b36201b5cb8e30c01edbd748786/third_party/WebKit/Source/modules/indexeddb/IDBRequestLoader.cpp [modify] https://crrev.com/e061f37ea9677b36201b5cb8e30c01edbd748786/third_party/WebKit/Source/modules/indexeddb/IDBRequestTest.cpp [modify] https://crrev.com/e061f37ea9677b36201b5cb8e30c01edbd748786/third_party/WebKit/Source/modules/indexeddb/IDBTransactionTest.cpp |
||||||||||
►
Sign in to add a comment |
||||||||||
Comment 1 by dmu...@chromium.org
, Oct 24 20175.7 KB
5.7 KB View Download