Commit 782ece68 authored by tzik's avatar tzik Committed by Commit Bot

Avoid touching ChainedBlobWriterImpl's ref count before it's fully constructed

ChainedBlobWriterImpl is a ref-counted object, and its first reference
used to be taken by base::BindOnce implicitly in its constructor. The
reference is posted to another thread, and released after the task has
run.
However, if the task finished before the constructor finished, the object
is destroyed immediately, and `new ChainedBlobWriterImpl` may return
a stale pointer.

This CL adds a static constructor to ChainedBlobWriterImpl to keep a
reference while the task is posted.

Bug: 866456
Change-Id: If8fee2e8944ce550e766b5269d47878f357db6c5
Reviewed-on: https://chromium-review.googlesource.com/1148098Reviewed-by: default avatarDaniel Murphy <dmurph@chromium.org>
Commit-Queue: Taiju Tsuiki <tzik@chromium.org>
Cr-Commit-Position: refs/heads/master@{#577638}
parent 0bfebe75
...@@ -1574,20 +1574,19 @@ class IndexedDBBackingStore::Transaction::ChainedBlobWriterImpl ...@@ -1574,20 +1574,19 @@ class IndexedDBBackingStore::Transaction::ChainedBlobWriterImpl
public: public:
typedef IndexedDBBackingStore::Transaction::WriteDescriptorVec typedef IndexedDBBackingStore::Transaction::WriteDescriptorVec
WriteDescriptorVec; WriteDescriptorVec;
ChainedBlobWriterImpl( static scoped_refptr<ChainedBlobWriterImpl> Create(
int64_t database_id, int64_t database_id,
IndexedDBBackingStore* backing_store, IndexedDBBackingStore* backing_store,
WriteDescriptorVec* blobs, WriteDescriptorVec* blobs,
scoped_refptr<IndexedDBBackingStore::BlobWriteCallback> callback) scoped_refptr<IndexedDBBackingStore::BlobWriteCallback> callback) {
: waiting_for_callback_(false), auto writer = base::WrapRefCounted(new ChainedBlobWriterImpl(
database_id_(database_id), database_id, backing_store, std::move(callback)));
backing_store_(backing_store), writer->blobs_.swap(*blobs);
callback_(callback), writer->iter_ = writer->blobs_.begin();
aborted_(false) {
blobs_.swap(*blobs);
iter_ = blobs_.begin();
backing_store->task_runner()->PostTask( backing_store->task_runner()->PostTask(
FROM_HERE, base::BindOnce(&ChainedBlobWriterImpl::WriteNextFile, this)); FROM_HERE,
base::BindOnce(&ChainedBlobWriterImpl::WriteNextFile, writer));
return writer;
} }
void set_delegate(std::unique_ptr<FileWriterDelegate> delegate) override { void set_delegate(std::unique_ptr<FileWriterDelegate> delegate) override {
...@@ -1623,6 +1622,15 @@ class IndexedDBBackingStore::Transaction::ChainedBlobWriterImpl ...@@ -1623,6 +1622,15 @@ class IndexedDBBackingStore::Transaction::ChainedBlobWriterImpl
} }
private: private:
ChainedBlobWriterImpl(
int64_t database_id,
IndexedDBBackingStore* backing_store,
scoped_refptr<IndexedDBBackingStore::BlobWriteCallback> callback)
: waiting_for_callback_(false),
database_id_(database_id),
backing_store_(backing_store),
callback_(callback),
aborted_(false) {}
~ChainedBlobWriterImpl() override {} ~ChainedBlobWriterImpl() override {}
void WriteNextFile() { void WriteNextFile() {
...@@ -3404,7 +3412,7 @@ void IndexedDBBackingStore::Transaction::WriteNewBlobs( ...@@ -3404,7 +3412,7 @@ void IndexedDBBackingStore::Transaction::WriteNewBlobs(
} }
// Creating the writer will start it going asynchronously. The transaction // Creating the writer will start it going asynchronously. The transaction
// can be destructed before the callback is triggered. // can be destructed before the callback is triggered.
chained_blob_writer_ = new ChainedBlobWriterImpl( chained_blob_writer_ = ChainedBlobWriterImpl::Create(
database_id_, backing_store_, new_files_to_write, database_id_, backing_store_, new_files_to_write,
new BlobWriteCallbackWrapper(ptr_factory_.GetWeakPtr(), this, callback)); new BlobWriteCallbackWrapper(ptr_factory_.GetWeakPtr(), this, callback));
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment