Skip to content

Commit a3b7558

Browse files
vasiliiCommit bot
authored andcommitted
Revert of Remove void** from disk_cache interface. (patchset crosswalk-project#30 id:650001 of https://codereview.chromium.org/542733002/)
Reason for revert: Introduced memory leaks on linux asan http://build.chromium.org/p/chromium.memory/builders/Linux%20ASan%20LSan%20Tests%20%281%29/builds/5892: Direct leak of 80 byte(s) in 2 object(s) allocated from: #0 0x501ccb in operator new(unsigned long) (/b/build/slave/Linux_ASan_LSan_Tests__1_/build/src/out/Release/net_unittests+0x501ccb) #1 0x309f8d7 in disk_cache::BackendImpl::OpenNextEntryImpl(void**) net/disk_cache/blockfile/backend_impl.cc:620:5 #2 0x30a02cd in disk_cache::BackendImpl::SyncOpenNextEntry(void**, disk_cache::Entry**) net/disk_cache/blockfile/backend_impl.cc:436:17 #3 0x30d49dd in disk_cache::BackendIO::ExecuteBackendOperation() net/disk_cache/blockfile/in_flight_backend_io.cc:248:17 #4 0x2e3b0bf in Run base/callback.h:401:12 crosswalk-project#5 0x2e3b0bf in base::debug::TaskAnnotator::RunTask(char const*, char const*, base::PendingTask const&) base/debug/task_annotator.cc:62 crosswalk-project#6 0x2dc22fc in base::MessageLoop::RunTask(base::PendingTask const&) base/message_loop/message_loop.cc:446:3 crosswalk-project#7 0x2dc33cc in DeferOrRunPendingTask base/message_loop/message_loop.cc:456:5 crosswalk-project#8 0x2dc33cc in base::MessageLoop::DoWork() base/message_loop/message_loop.cc:565 crosswalk-project#9 0x2e2361f in base::MessagePumpLibevent::Run(base::MessagePump::Delegate*) base/message_loop/message_pump_libevent.cc:232:21 crosswalk-project#10 0x2ddd94b in base::RunLoop::Run() base/run_loop.cc:54:3 crosswalk-project#11 0x2dc0bc4 in base::MessageLoop::Run() base/message_loop/message_loop.cc:308:3 crosswalk-project#12 0x2e0ad90 in base::Thread::ThreadMain() base/threading/thread.cc:228:5 crosswalk-project#13 0x2dfeaf0 in base::(anonymous namespace)::ThreadFunc(void*) base/threading/platform_thread_posix.cc:80:3 crosswalk-project#14 0x7fa9f4e0ce99 in start_thread /build/buildd/eglibc-2.15/nptl/pthread_create.c:308 Indirect leak of 64 byte(s) in 2 object(s) allocated from: #0 0x501ccb in operator new(unsigned long) (/b/build/slave/Linux_ASan_LSan_Tests__1_/build/src/out/Release/net_unittests+0x501ccb) #1 0x30ebaaf in disk_cache::Rankings::GetNext(disk_cache::StorageBlock<disk_cache::RankingsNode>*, disk_cache::Rankings::List) net/disk_cache/blockfile/rankings.cc:435:5 #2 0x30a219d in disk_cache::BackendImpl::OpenFollowingEntryFromList(disk_cache::Rankings::List, disk_cache::StorageBlock<disk_cache::RankingsNode>**, disk_cache::EntryImpl**) net/disk_cache/blockfile/backend_impl.cc:1694:36 #3 0x309fa3d in disk_cache::BackendImpl::OpenNextEntryImpl(void**) net/disk_cache/blockfile/backend_impl.cc:638:11 #4 0x30a02cd in disk_cache::BackendImpl::SyncOpenNextEntry(void**, disk_cache::Entry**) net/disk_cache/blockfile/backend_impl.cc:436:17 crosswalk-project#5 0x30d49dd in disk_cache::BackendIO::ExecuteBackendOperation() net/disk_cache/blockfile/in_flight_backend_io.cc:248:17 crosswalk-project#6 0x2e3b0bf in Run base/callback.h:401:12 crosswalk-project#7 0x2e3b0bf in base::debug::TaskAnnotator::RunTask(char const*, char const*, base::PendingTask const&) base/debug/task_annotator.cc:62 crosswalk-project#8 0x2dc22fc in base::MessageLoop::RunTask(base::PendingTask const&) base/message_loop/message_loop.cc:446:3 crosswalk-project#9 0x2dc33cc in DeferOrRunPendingTask base/message_loop/message_loop.cc:456:5 crosswalk-project#10 0x2dc33cc in base::MessageLoop::DoWork() base/message_loop/message_loop.cc:565 crosswalk-project#11 0x2e2361f in base::MessagePumpLibevent::Run(base::MessagePump::Delegate*) base/message_loop/message_pump_libevent.cc:232:21 crosswalk-project#12 0x2ddd94b in base::RunLoop::Run() base/run_loop.cc:54:3 crosswalk-project#13 0x2dc0bc4 in base::MessageLoop::Run() base/message_loop/message_loop.cc:308:3 crosswalk-project#14 0x2e0ad90 in base::Thread::ThreadMain() base/threading/thread.cc:228:5 crosswalk-project#15 0x2dfeaf0 in base::(anonymous namespace)::ThreadFunc(void*) base/threading/platform_thread_posix.cc:80:3 crosswalk-project#16 0x7fa9f4e0ce99 in start_thread /build/buildd/eglibc-2.15/nptl/pthread_create.c:308 Indirect leak of 36 byte(s) in 1 object(s) allocated from: #0 0x501ccb in operator new(unsigned long) (/b/build/slave/Linux_ASan_LSan_Tests__1_/build/src/out/Release/net_unittests+0x501ccb) #1 0x30c0696 in AllocateData net/disk_cache/blockfile/storage_block-inl.h:179:5 #2 0x30c0696 in disk_cache::StorageBlock<disk_cache::RankingsNode>::Load() net/disk_cache/blockfile/storage_block-inl.h:121 #3 0x30e6fe9 in disk_cache::Rankings::GetRanking(disk_cache::StorageBlock<disk_cache::RankingsNode>*) net/disk_cache/blockfile/rankings.cc:586:8 #4 0x30ebd5a in disk_cache::Rankings::GetNext(disk_cache::StorageBlock<disk_cache::RankingsNode>*, disk_cache::Rankings::List) net/disk_cache/blockfile/rankings.cc:440:8 crosswalk-project#5 0x30a219d in disk_cache::BackendImpl::OpenFollowingEntryFromList(disk_cache::Rankings::List, disk_cache::StorageBlock<disk_cache::RankingsNode>**, disk_cache::EntryImpl**) net/disk_cache/blockfile/backend_impl.cc:1694:36 crosswalk-project#6 0x309fa3d in disk_cache::BackendImpl::OpenNextEntryImpl(void**) net/disk_cache/blockfile/backend_impl.cc:638:11 crosswalk-project#7 0x30a02cd in disk_cache::BackendImpl::SyncOpenNextEntry(void**, disk_cache::Entry**) net/disk_cache/blockfile/backend_impl.cc:436:17 crosswalk-project#8 0x30d49dd in disk_cache::BackendIO::ExecuteBackendOperation() net/disk_cache/blockfile/in_flight_backend_io.cc:248:17 crosswalk-project#9 0x2e3b0bf in Run base/callback.h:401:12 crosswalk-project#10 0x2e3b0bf in base::debug::TaskAnnotator::RunTask(char const*, char const*, base::PendingTask const&) base/debug/task_annotator.cc:62 crosswalk-project#11 0x2dc22fc in base::MessageLoop::RunTask(base::PendingTask const&) base/message_loop/message_loop.cc:446:3 crosswalk-project#12 0x2dc33cc in DeferOrRunPendingTask base/message_loop/message_loop.cc:456:5 crosswalk-project#13 0x2dc33cc in base::MessageLoop::DoWork() base/message_loop/message_loop.cc:565 crosswalk-project#14 0x2e2361f in base::MessagePumpLibevent::Run(base::MessagePump::Delegate*) base/message_loop/message_pump_libevent.cc:232:21 crosswalk-project#15 0x2ddd94b in base::RunLoop::Run() base/run_loop.cc:54:3 crosswalk-project#16 0x2dc0bc4 in base::MessageLoop::Run() base/message_loop/message_loop.cc:308:3 crosswalk-project#17 0x2e0ad90 in base::Thread::ThreadMain() base/threading/thread.cc:228:5 crosswalk-project#18 0x2dfeaf0 in base::(anonymous namespace)::ThreadFunc(void*) base/threading/platform_thread_posix.cc:80:3 crosswalk-project#19 0x7fa9f4e0ce99 in start_thread /build/buildd/eglibc-2.15/nptl/pthread_create.c:308 Original issue's description: > Remove void** from disk_cache interface. > > Enumeration and iteration were passing around void**. With this CL, we > instead use an Iterator object. > > [email protected],[email protected],[email protected] > BUG=413644 > > Committed: https://crrev.com/732c8306d4864296511e7a3a252724b1bb34c342 > Cr-Commit-Position: refs/heads/master@{#295659} [email protected],[email protected],[email protected],[email protected],[email protected],[email protected],[email protected],[email protected],[email protected] NOTREECHECKS=true NOTRY=true BUG=413644 Review URL: https://codereview.chromium.org/585833002 Cr-Commit-Position: refs/heads/master@{#295677}
1 parent 48c3e5e commit a3b7558

21 files changed

+346
-353
lines changed

content/browser/gpu/shader_disk_cache.cc

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,7 @@ class ShaderDiskReadHelper
9595

9696
base::WeakPtr<ShaderDiskCache> cache_;
9797
OpType op_type_;
98-
scoped_ptr<disk_cache::Backend::Iterator> iter_;
98+
void* iter_;
9999
scoped_refptr<net::IOBufferWithSize> buf_;
100100
int host_id_;
101101
disk_cache::Entry* entry_;
@@ -243,6 +243,7 @@ ShaderDiskReadHelper::ShaderDiskReadHelper(
243243
int host_id)
244244
: cache_(cache),
245245
op_type_(OPEN_NEXT),
246+
iter_(NULL),
246247
buf_(NULL),
247248
host_id_(host_id),
248249
entry_(NULL) {
@@ -290,10 +291,10 @@ int ShaderDiskReadHelper::OpenNextEntry() {
290291
DCHECK(CalledOnValidThread());
291292
// Called through OnOpComplete, so we know |cache_| is valid.
292293
op_type_ = OPEN_NEXT_COMPLETE;
293-
if (!iter_)
294-
iter_ = cache_->backend()->CreateIterator();
295-
return iter_->OpenNextEntry(
296-
&entry_, base::Bind(&ShaderDiskReadHelper::OnOpComplete, this));
294+
return cache_->backend()->OpenNextEntry(
295+
&iter_,
296+
&entry_,
297+
base::Bind(&ShaderDiskReadHelper::OnOpComplete, this));
297298
}
298299

299300
int ShaderDiskReadHelper::OpenNextEntryComplete(int rv) {
@@ -338,7 +339,8 @@ int ShaderDiskReadHelper::ReadComplete(int rv) {
338339
int ShaderDiskReadHelper::IterationComplete(int rv) {
339340
DCHECK(CalledOnValidThread());
340341
// Called through OnOpComplete, so we know |cache_| is valid.
341-
iter_.reset();
342+
cache_->backend()->EndEnumeration(&iter_);
343+
iter_ = NULL;
342344
op_type_ = TERMINATE;
343345
return net::OK;
344346
}

content/browser/service_worker/service_worker_cache.cc

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -597,13 +597,16 @@ struct ServiceWorkerCache::KeysContext {
597597
: original_callback(callback),
598598
cache(cache),
599599
out_keys(new ServiceWorkerCache::Requests()),
600+
backend_iterator(NULL),
600601
enumerated_entry(NULL) {}
601602

602603
~KeysContext() {
603604
for (size_t i = 0, max = entries.size(); i < max; ++i)
604605
entries[i]->Close();
605606
if (enumerated_entry)
606607
enumerated_entry->Close();
608+
if (cache && backend_iterator && cache->backend_)
609+
cache->backend_->EndEnumeration(&backend_iterator);
607610
}
608611

609612
// The callback passed to the Keys() function.
@@ -619,7 +622,7 @@ struct ServiceWorkerCache::KeysContext {
619622
scoped_ptr<ServiceWorkerCache::Requests> out_keys;
620623

621624
// Used for enumerating cache entries.
622-
scoped_ptr<disk_cache::Backend::Iterator> backend_iterator;
625+
void* backend_iterator;
623626
disk_cache::Entry* enumerated_entry;
624627
};
625628

@@ -771,14 +774,14 @@ void ServiceWorkerCache::Keys(const RequestsCallback& callback) {
771774
scoped_ptr<KeysContext> keys_context(
772775
new KeysContext(callback, weak_ptr_factory_.GetWeakPtr()));
773776

774-
keys_context->backend_iterator = backend_->CreateIterator();
775-
disk_cache::Backend::Iterator& iterator = *keys_context->backend_iterator;
777+
void** backend_iterator = &keys_context->backend_iterator;
776778
disk_cache::Entry** enumerated_entry = &keys_context->enumerated_entry;
777779

778780
net::CompletionCallback open_entry_callback =
779781
base::Bind(KeysDidOpenNextEntry, base::Passed(keys_context.Pass()));
780782

781-
int rv = iterator.OpenNextEntry(enumerated_entry, open_entry_callback);
783+
int rv = backend_->OpenNextEntry(
784+
backend_iterator, enumerated_entry, open_entry_callback);
782785

783786
if (rv != net::ERR_IO_PENDING)
784787
open_entry_callback.Run(rv);
@@ -861,12 +864,14 @@ void ServiceWorkerCache::KeysDidOpenNextEntry(
861864
keys_context->enumerated_entry = NULL;
862865

863866
// Enumerate the next entry.
864-
disk_cache::Backend::Iterator& iterator = *keys_context->backend_iterator;
867+
void** backend_iterator = &keys_context->backend_iterator;
865868
disk_cache::Entry** enumerated_entry = &keys_context->enumerated_entry;
869+
866870
net::CompletionCallback open_entry_callback =
867871
base::Bind(KeysDidOpenNextEntry, base::Passed(keys_context.Pass()));
868872

869-
rv = iterator.OpenNextEntry(enumerated_entry, open_entry_callback);
873+
rv = cache->backend_->OpenNextEntry(
874+
backend_iterator, enumerated_entry, open_entry_callback);
870875

871876
if (rv != net::ERR_IO_PENDING)
872877
open_entry_callback.Run(rv);

0 commit comments

Comments
 (0)