IndexedDB blob unwrapping leaks blob urls when reading a getAll. |
|||
Issue description
The FileReaderLoader.cpp doesn't clean up old data or the url_for_reading_ blob url when being asked to 'Start' again. Our IDBRequestLoader tries to re-use that object, so when we're doing a getAll operation with multiple wrapped blobs to process, we will leak all URLs except the last one.
The FileReaderLoader.cpp should check to see if that URL is still there, and probably do something like this:
if (!url_for_reading_.IsEmpty()) {
ClearData();
BlobRegistry::RevokePublicBlobURL(url_for_reading_);
}
where ClearData is:
void FileReaderLoader::ClearData() {
raw_data_.reset();
string_result_ = "";
is_raw_data_converted_ = true;
decoder_.reset();
array_buffer_result_ = nullptr;
UnadjustReportedMemoryUsageToV8();
}
,
Oct 24 2017
patch with the change (and other debugging): https://chromium-review.googlesource.com/#/c/chromium/src/+/734642
,
Oct 24 2017
Or maybe we should just clarify the documentation of FileReaderLoader that you're not supposed to try to reuse it, and fix the code to not reuse it?
,
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 4 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/df6f1e38cf36e9cf968377f810a7064e93b690ce commit df6f1e38cf36e9cf968377f810a7064e93b690ce Author: Victor Costan <pwnall@chromium.org> Date: Sat Nov 04 04:19:45 2017 File API: Clarify (with check) that FileReaderLoader is single-use. Bug: 777665 Change-Id: I4cbefece39e1c87bb80ae8444f5484bacb06d1a7 Reviewed-on: https://chromium-review.googlesource.com/754040 Reviewed-by: Daniel Murphy <dmurph@chromium.org> Commit-Queue: Victor Costan <pwnall@chromium.org> Cr-Commit-Position: refs/heads/master@{#514028} [modify] https://crrev.com/df6f1e38cf36e9cf968377f810a7064e93b690ce/third_party/WebKit/Source/core/fileapi/FileReaderLoader.cpp [modify] https://crrev.com/df6f1e38cf36e9cf968377f810a7064e93b690ce/third_party/WebKit/Source/core/fileapi/FileReaderLoader.h
,
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
,
Nov 30 2017
|
|||
►
Sign in to add a comment |
|||
Comment 1 Deleted