[debugger] Retry downloads when new servers appear
Change-Id: Ib85730783f9c83fa2c74c11f68fa1a8916bbccc7
diff --git a/src/developer/debug/zxdb/client/symbol_server.cc b/src/developer/debug/zxdb/client/symbol_server.cc
index 2957525..9f72465 100644
--- a/src/developer/debug/zxdb/client/symbol_server.cc
+++ b/src/developer/debug/zxdb/client/symbol_server.cc
@@ -336,7 +336,7 @@
state_ = state;
if (state_change_callback_)
- state_change_callback_(state_);
+ state_change_callback_(this, state_);
}
std::unique_ptr<SymbolServer> SymbolServer::FromURL(Session* session,
diff --git a/src/developer/debug/zxdb/client/symbol_server.h b/src/developer/debug/zxdb/client/symbol_server.h
index 7b7e55c..07bbe13 100644
--- a/src/developer/debug/zxdb/client/symbol_server.h
+++ b/src/developer/debug/zxdb/client/symbol_server.h
@@ -43,7 +43,7 @@
const std::vector<std::string>& error_log() const { return error_log_; }
State state() const { return state_; }
- void set_state_change_callback(std::function<void(State)> cb) {
+ void set_state_change_callback(std::function<void(SymbolServer*,State)> cb) {
state_change_callback_ = cb;
}
@@ -80,7 +80,7 @@
// handle custom protocol identifiers etc.
std::string name_;
- std::function<void(State)> state_change_callback_ = nullptr;
+ std::function<void(SymbolServer*,State)> state_change_callback_ = nullptr;
};
} // namespace zxdb
diff --git a/src/developer/debug/zxdb/client/system_impl.cc b/src/developer/debug/zxdb/client/system_impl.cc
index 0fa87d0..b7778fa 100644
--- a/src/developer/debug/zxdb/client/system_impl.cc
+++ b/src/developer/debug/zxdb/client/system_impl.cc
@@ -18,11 +18,11 @@
#include "src/developer/debug/zxdb/client/system_observer.h"
#include "src/developer/debug/zxdb/client/target_impl.h"
#include "src/developer/debug/zxdb/common/string_util.h"
+#include "src/developer/debug/zxdb/symbols/loaded_module_symbols.h"
#include "src/developer/debug/zxdb/symbols/module_symbol_status.h"
#include "src/lib/fxl/strings/string_printf.h"
namespace zxdb {
-namespace {
// When we want to download symbols for a build ID, we create a Download
// object. We then fire off requests to all symbol servers we know about asking
@@ -60,6 +60,9 @@
// Notify this Download object that a transaction failed.
void Error(std::shared_ptr<Download> self, const Err& err);
+ // Add a symbol server to this download.
+ void AddServer(std::shared_ptr<Download> self, SymbolServer* server);
+
private:
void RunCB(std::shared_ptr<Download> self,
std::function<void(SymbolServer::FetchCallback)>& cb);
@@ -72,6 +75,20 @@
bool trying_ = false;
};
+void Download::AddServer(std::shared_ptr<Download> self, SymbolServer* server) {
+ FXL_DCHECK(self.get() == this);
+
+ server->CheckFetch(
+ build_id_,
+ [self](const Err& err,
+ std::function<void(SymbolServer::FetchCallback)> cb) {
+ if (!cb)
+ self->Error(self, err);
+ else
+ self->Found(self, std::move(cb));
+ });
+}
+
void Download::Found(std::shared_ptr<Download> self,
std::function<void(SymbolServer::FetchCallback)> cb) {
FXL_DCHECK(self.get() == this);
@@ -116,8 +133,6 @@
});
}
-} // namespace
-
SystemImpl::SystemImpl(Session* session)
: System(session), symbols_(this), weak_factory_(this) {
// Create the default job and target.
@@ -231,9 +246,13 @@
return result;
}
-void SystemImpl::RequestDownload(const std::string& build_id) {
+std::shared_ptr<Download> SystemImpl::GetDownload(std::string build_id, bool quiet) {
+ if (auto existing = downloads_[build_id].lock()) {
+ return existing;
+ }
+
auto download = std::make_shared<Download>(
- build_id, [build_id, weak_this = weak_factory_.GetWeakPtr()](
+ build_id, [build_id, weak_this = weak_factory_.GetWeakPtr(), quiet](
const Err& err, const std::string& path) {
if (!weak_this) {
return;
@@ -253,25 +272,25 @@
process->GetSymbols()->RetryLoadBuildID(build_id);
}
}
- } else {
+ } else if (!quiet) {
weak_this->NotifyFailedToFindDebugSymbols(err, build_id);
}
});
+ downloads_[build_id] = download;
+
+ return download;
+}
+
+void SystemImpl::RequestDownload(const std::string& build_id, bool quiet) {
+ auto download = GetDownload(build_id, quiet);
+
for (auto& server : symbol_servers_) {
if (server->state() != SymbolServer::State::kReady) {
continue;
}
- server->CheckFetch(
- build_id,
- [download](const Err& err,
- std::function<void(SymbolServer::FetchCallback)> cb) {
- if (!cb)
- download->Error(download, err);
- else
- download->Found(download, std::move(cb));
- });
+ download->AddServer(download, server.get());
}
}
@@ -300,6 +319,21 @@
}
}
+void SystemImpl::OnSymbolServerBecomesReady(SymbolServer* server) {
+ for (const auto& target : targets_) {
+ auto process = target->process();
+ if (!process)
+ continue;
+
+ for (const auto& mod : process->GetSymbols()->GetStatus()) {
+ if (!mod.symbols || !mod.symbols->module_ref()) {
+ auto download = GetDownload(mod.build_id, true);
+ download->AddServer(download, server);
+ }
+ }
+ }
+}
+
Process* SystemImpl::ProcessFromKoid(uint64_t koid) const {
return ProcessImplFromKoid(koid);
}
@@ -467,9 +501,21 @@
for (const auto& url : urls) {
if (existing.find(url) == existing.end()) {
symbol_servers_.push_back(SymbolServer::FromURL(session(), url));
+ auto& server = symbol_servers_.back();
for (auto& observer : observers()) {
- observer.DidCreateSymbolServer(symbol_servers_.back().get());
+ observer.DidCreateSymbolServer(server.get());
+ }
+
+ server->set_state_change_callback(
+ [weak_this = weak_factory_.GetWeakPtr()](
+ SymbolServer* server, SymbolServer::State state) {
+ if (weak_this && state == SymbolServer::State::kReady)
+ weak_this->OnSymbolServerBecomesReady(server);
+ });
+
+ if (server->state() == SymbolServer::State::kReady) {
+ OnSymbolServerBecomesReady(server.get());
}
}
}
diff --git a/src/developer/debug/zxdb/client/system_impl.h b/src/developer/debug/zxdb/client/system_impl.h
index 0615f75..b1ec371 100644
--- a/src/developer/debug/zxdb/client/system_impl.h
+++ b/src/developer/debug/zxdb/client/system_impl.h
@@ -17,10 +17,11 @@
namespace zxdb {
class BreakpointImpl;
+class Download;
+class JobContextImpl;
class ProcessImpl;
class SystemSymbolsProxy;
class TargetImpl;
-class JobContextImpl;
class SystemImpl final : public System,
public SettingStoreObserver,
@@ -57,7 +58,7 @@
void Continue() override;
// DownloadHandler implementation:
- void RequestDownload(const std::string& build_id) override;
+ void RequestDownload(const std::string& build_id, bool quiet) override;
// Notification that a connection has been made/terminated to a target
// system.
@@ -83,10 +84,24 @@
void NotifyFailedToFindDebugSymbols(const Err& err,
const std::string& build_id);
+ // Called when a symbol server under our control enters the Ready state.
+ void OnSymbolServerBecomesReady(SymbolServer* server);
+
+ // Create a new download obect for downloading a given build ID. If quiet is
+ // set, don't report the status of this download.
+ //
+ // If multiple callers request a download of the same build ID, this will
+ // return the same object to each. The first caller's preference is taken for
+ // the quiet parameter.
+ std::shared_ptr<Download> GetDownload(std::string build_id, bool quiet);
+
std::vector<std::unique_ptr<SymbolServer>> symbol_servers_;
std::vector<std::unique_ptr<TargetImpl>> targets_;
std::vector<std::unique_ptr<JobContextImpl>> job_contexts_;
+ // Downloads currently in progress.
+ std::map<std::string, std::weak_ptr<Download>> downloads_;
+
// The breakpoints are indexed by their unique backend ID. This is separate
// from the index generated by the console frontend to describe the
// breakpoint noun.
diff --git a/src/developer/debug/zxdb/symbols/system_symbols.cc b/src/developer/debug/zxdb/symbols/system_symbols.cc
index 17b518c..87ca16f 100644
--- a/src/developer/debug/zxdb/symbols/system_symbols.cc
+++ b/src/developer/debug/zxdb/symbols/system_symbols.cc
@@ -90,7 +90,7 @@
FXL_DCHECK(download_handler_);
*module = nullptr;
- download_handler_->RequestDownload(build_id);
+ download_handler_->RequestDownload(build_id, false);
return Err();
}
diff --git a/src/developer/debug/zxdb/symbols/system_symbols.h b/src/developer/debug/zxdb/symbols/system_symbols.h
index dbe6af4..8ec759d 100644
--- a/src/developer/debug/zxdb/symbols/system_symbols.h
+++ b/src/developer/debug/zxdb/symbols/system_symbols.h
@@ -53,7 +53,7 @@
class DownloadHandler {
public:
- virtual void RequestDownload(const std::string& build_id) = 0;
+ virtual void RequestDownload(const std::string& build_id, bool quiet) = 0;
};
explicit SystemSymbols(DownloadHandler* download_handler);
@@ -85,7 +85,7 @@
// Request that the system arrange for symbols to be downloaded for a given
// build ID.
- void RequestDownload(const std::string& build_id);
+ void RequestDownload(const std::string& build_id, bool quiet);
// Notification from the ModuleRef that all references have been deleted and
// the tracking information should be removed from the map.
