Commit aba55d24 authored by Konstantin Ganenko's avatar Konstantin Ganenko Committed by Commit Bot

Fix using CompletionHandler after deleting

CompletionHandler is used in CacheThread_BlockFile.
CacheThread_BlockFile is lazy + leaky instance, thus, there was race
 condition when all lazy instances are already destroyed, but message
 io pump still processes some io overlapped operation,leading to using
 IOHandler after free.
Make completion handler to be refcounted and add ref to each overlapped
 operation it participates with.

R=jkarlin@chromium.org, morlovich@chromium.org

Bug: 
Change-Id: I7bd5fbdc83aae05489d5089ee16a94857ea1b104
Reviewed-on: https://chromium-review.googlesource.com/793158Reviewed-by: default avatarMaks Orlovich <morlovich@chromium.org>
Commit-Queue: Maks Orlovich <morlovich@chromium.org>
Cr-Commit-Position: refs/heads/master@{#523556}
parent 27baffb1
......@@ -15,6 +15,7 @@
namespace {
class CompletionHandler;
// Structure used for asynchronous operations.
struct MyOverlapped {
MyOverlapped(disk_cache::File* file, size_t offset,
......@@ -26,6 +27,7 @@ struct MyOverlapped {
base::MessageLoopForIO::IOContext context_;
scoped_refptr<disk_cache::File> file_;
scoped_refptr<CompletionHandler> completion_handler_;
disk_cache::FileIOCallback* callback_;
};
......@@ -33,14 +35,43 @@ static_assert(offsetof(MyOverlapped, context_) == 0,
"should start with overlapped");
// Helper class to handle the IO completion notifications from the message loop.
class CompletionHandler : public base::MessageLoopForIO::IOHandler {
class CompletionHandler : public base::MessageLoopForIO::IOHandler,
public base::RefCounted<CompletionHandler> {
public:
CompletionHandler() = default;
static CompletionHandler* Get();
private:
friend class base::RefCounted<CompletionHandler>;
~CompletionHandler() override {}
// implement base::MessageLoopForIO::IOHandler.
void OnIOCompleted(base::MessageLoopForIO::IOContext* context,
DWORD actual_bytes,
DWORD error) override;
DISALLOW_COPY_AND_ASSIGN(CompletionHandler);
};
class CompletionHandlerHolder {
public:
CompletionHandlerHolder() { completion_handler_ = new CompletionHandler; }
CompletionHandler* completion_handler() { return completion_handler_.get(); }
private:
scoped_refptr<CompletionHandler> completion_handler_;
};
static base::LazyInstance<CompletionHandler>::DestructorAtExit
g_completion_handler = LAZY_INSTANCE_INITIALIZER;
static base::LazyInstance<CompletionHandlerHolder>::DestructorAtExit
g_completion_handler_holder = LAZY_INSTANCE_INITIALIZER;
CompletionHandler* CompletionHandler::Get() {
if (auto* holder = g_completion_handler_holder.Pointer()) {
return holder->completion_handler();
}
return nullptr;
}
void CompletionHandler::OnIOCompleted(
base::MessageLoopForIO::IOContext* context,
......@@ -65,6 +96,7 @@ MyOverlapped::MyOverlapped(disk_cache::File* file, size_t offset,
context_.overlapped.Offset = static_cast<DWORD>(offset);
file_ = file;
callback_ = callback;
completion_handler_ = CompletionHandler::Get();
}
} // namespace
......@@ -89,7 +121,7 @@ bool File::Init(const base::FilePath& name) {
return false;
base::MessageLoopForIO::current()->RegisterIOHandler(
base_file_.GetPlatformFile(), g_completion_handler.Pointer());
base_file_.GetPlatformFile(), CompletionHandler::Get());
init_ = true;
sync_base_file_ =
......@@ -244,7 +276,7 @@ void File::WaitForPendingIO(int* num_pending_io) {
while (*num_pending_io) {
// Asynchronous IO operations may be in flight and the completion may end
// up calling us back so let's wait for them.
base::MessageLoopForIO::IOHandler* handler = g_completion_handler.Pointer();
base::MessageLoopForIO::IOHandler* handler = CompletionHandler::Get();
base::MessageLoopForIO::current()->WaitForIOCompletion(100, handler);
}
}
......
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