From 662a31babf0c006cd5c366cef8f982143fd2d72a Mon Sep 17 00:00:00 2001
From: Casey Dahlin <sadmac@google.com>
Date: Mon, 22 Apr 2019 19:35:54 +0000
Subject: [PATCH] [debugger] Run Download callback earlier
Previously, the Download callback wouldn't run until all pending
requests had finished. This means all the servers we initially query
must at least respond to the initial check request before we get the
results of our download. With this patch we run the callback as soon as
we have the symbols down. This should also prevent a potential race
where server checks that fail after we've already gotten the symbols
could change our error status.
Change-Id: I7db38e1b5593921594792d39e42164cd0184b4c6
---
.../debug/zxdb/client/system_impl.cc | 29 +++++++++++++++++--
1 file changed, 26 insertions(+), 3 deletions(-)
diff --git a/src/developer/debug/zxdb/client/system_impl.cc b/src/developer/debug/zxdb/client/system_impl.cc
index 2d8a1c62bc4..9d57c1882ba 100644
--- a/src/developer/debug/zxdb/client/system_impl.cc
+++ b/src/developer/debug/zxdb/client/system_impl.cc
@@ -48,11 +48,13 @@ class Download {
: build_id_(build_id), result_cb_(std::move(result_cb)) {}
~Download() {
- debug_ipc::MessageLoop::Current()->PostTask(
- FROM_HERE, [result_cb = std::move(result_cb_), err = std::move(err_),
- path = std::move(path_)]() { result_cb(err, path); });
+ Finish();
}
+ // Notify this download object that we have gotten the symbols if we're going
+ // to get them.
+ void Finish();
+
// Notify this Download object that one of the servers has the symbols
// available.
void Found(std::shared_ptr<Download> self,
@@ -76,9 +78,23 @@ class Download {
bool trying_ = false;
};
+void Download::Finish() {
+ if (!result_cb_)
+ return;
+
+ debug_ipc::MessageLoop::Current()->PostTask(
+ FROM_HERE, [result_cb = std::move(result_cb_), err = std::move(err_),
+ path = std::move(path_)]() { result_cb(err, path); });
+
+ result_cb_ = nullptr;
+}
+
void Download::AddServer(std::shared_ptr<Download> self, SymbolServer* server) {
FXL_DCHECK(self.get() == this);
+ if (!result_cb_)
+ return;
+
server->CheckFetch(
build_id_,
[self](const Err& err,
@@ -94,6 +110,9 @@ void Download::Found(std::shared_ptr<Download> self,
std::function<void(SymbolServer::FetchCallback)> cb) {
FXL_DCHECK(self.get() == this);
+ if (!result_cb_)
+ return;
+
if (trying_) {
server_cbs_.push_back(std::move(cb));
return;
@@ -105,6 +124,9 @@ void Download::Found(std::shared_ptr<Download> self,
void Download::Error(std::shared_ptr<Download> self, const Err& err) {
FXL_DCHECK(self.get() == this);
+ if (!result_cb_)
+ return;
+
if (!err_.has_error()) {
err_ = err;
} else if (err.has_error()) {
@@ -130,6 +152,7 @@ void Download::RunCB(std::shared_ptr<Download> self,
} else {
self->err_ = err;
self->path_ = path;
+ self->Finish();
}
});
}
--
GitLab