[debugger] Make debug agent persists upon disconnects.
Before the debug agent would be bound to a connection, so the moment the
connection got lost, the agent would be destroyed and all the
attachments/breakpoints would be lost, somewhat defeating the purpose of
reconnecting.
With this change, the agent works independent of a connection, so can
connect/disconnect multiple times without loosing state.
IMPORTANT: While the agent doesn't loose state, this setup will actually
"leak" as the newly connected agent won't know about the current
state of the debug agent (it assumes a "clean" debug agent).
While this is somewhat a regression, it's not that bad as
the default setup is to quit on exit so the agent will exit
anyway upon disconnection.
That change will come in a subsequent CL(s).
TEST=Manual with debug mode activated agent and using `quit-on-exit`
false config, which enables an agent to remain upon disconnection.
Change-Id: If78776a9a215e42613446ec773b5ccfd24c7cfee
diff --git a/src/developer/debug/debug_agent/debug_agent.cc b/src/developer/debug/debug_agent/debug_agent.cc
index 48a60a1..26e53e4 100644
--- a/src/developer/debug/debug_agent/debug_agent.cc
+++ b/src/developer/debug/debug_agent/debug_agent.cc
@@ -8,6 +8,7 @@
#include <lib/async-loop/cpp/loop.h>
#include <lib/sys/cpp/termination_reason.h>
#include <zircon/features.h>
+#include <zircon/status.h>
#include <zircon/syscalls/debug.h>
#include <zircon/syscalls/exception.h>
@@ -39,14 +40,28 @@
} // namespace
-DebugAgent::DebugAgent(debug_ipc::StreamBuffer* stream,
- std::shared_ptr<sys::ServiceDirectory> services)
- : stream_(stream), services_(services), weak_factory_(this) {}
+DebugAgent::DebugAgent(std::shared_ptr<sys::ServiceDirectory> services)
+ : services_(services), weak_factory_(this) {}
DebugAgent::~DebugAgent() = default;
fxl::WeakPtr<DebugAgent> DebugAgent::GetWeakPtr() { return weak_factory_.GetWeakPtr(); }
+void DebugAgent::Connect(debug_ipc::StreamBuffer* stream) {
+ FXL_DCHECK(!stream_) << "A debug agent should not be connected twice!";
+ stream_ = stream;
+}
+
+void DebugAgent::Disconnect() {
+ FXL_DCHECK(stream_);
+ stream_ = nullptr;
+}
+
+debug_ipc::StreamBuffer* DebugAgent::stream() {
+ FXL_DCHECK(stream_);
+ return stream_;
+}
+
void DebugAgent::RemoveDebuggedProcess(zx_koid_t process_koid) {
auto found = procs_.find(process_koid);
if (found == procs_.end())
@@ -160,6 +175,8 @@
reply.status = AddDebuggedJob(job_koid, std::move(job));
}
+ DEBUG_LOG(Agent) << "Attaching to job " << job_koid << ": " << zx_status_get_string(reply.status);
+
// Send the reply.
debug_ipc::MessageWriter writer;
debug_ipc::WriteReply(reply, transaction_id, &writer);
diff --git a/src/developer/debug/debug_agent/debug_agent.h b/src/developer/debug/debug_agent/debug_agent.h
index b816a35..8f7cba1 100644
--- a/src/developer/debug/debug_agent/debug_agent.h
+++ b/src/developer/debug/debug_agent/debug_agent.h
@@ -35,13 +35,18 @@
// The stream must outlive this class. It will be used to send data to the
// client. It will not be read (that's the job of the provider of the
// RemoteAPI).
- explicit DebugAgent(debug_ipc::StreamBuffer* stream,
- std::shared_ptr<sys::ServiceDirectory> services);
+ explicit DebugAgent(std::shared_ptr<sys::ServiceDirectory> services);
~DebugAgent();
fxl::WeakPtr<DebugAgent> GetWeakPtr();
- debug_ipc::StreamBuffer* stream() { return stream_; }
+ // Connects the debug agent to a stream buffer.
+ // The buffer can be disconnected and the debug agent will remain intact until the moment a new
+ // buffer is connected and messages start flowing through again.
+ void Connect(debug_ipc::StreamBuffer*);
+ void Disconnect();
+
+ debug_ipc::StreamBuffer* stream();
void RemoveDebuggedProcess(zx_koid_t process_koid);
@@ -132,7 +137,7 @@
void OnComponentTerminated(int64_t return_code, const ComponentDescription& description,
fuchsia::sys::TerminationReason reason);
- debug_ipc::StreamBuffer* stream_;
+ debug_ipc::StreamBuffer* stream_ = nullptr;
std::shared_ptr<sys::ServiceDirectory> services_;
diff --git a/src/developer/debug/debug_agent/integration_tests/mock_stream_backend.cc b/src/developer/debug/debug_agent/integration_tests/mock_stream_backend.cc
index bba6e135..0567e2d 100644
--- a/src/developer/debug/debug_agent/integration_tests/mock_stream_backend.cc
+++ b/src/developer/debug/debug_agent/integration_tests/mock_stream_backend.cc
@@ -16,7 +16,8 @@
// think it's correctly connected to a client.
stream_.set_writer(this);
auto services = sys::ServiceDirectory::CreateFromNamespace();
- agent_ = std::make_unique<DebugAgent>(&stream_, std::move(services));
+ agent_ = std::make_unique<DebugAgent>(std::move(services));
+ agent_->Connect(&stream_);
}
size_t MockStreamBackend::ConsumeStreamBufferData(const char* data, size_t len) {
diff --git a/src/developer/debug/debug_agent/main.cc b/src/developer/debug/debug_agent/main.cc
index 26888b0..7baaea5 100644
--- a/src/developer/debug/debug_agent/main.cc
+++ b/src/developer/debug/debug_agent/main.cc
@@ -10,6 +10,7 @@
#include <memory>
#include <thread>
+#include "src/developer/debug/debug_agent/debug_agent.h"
#include "src/developer/debug/debug_agent/socket_connection.h"
#include "src/developer/debug/debug_agent/unwind.h"
#include "src/developer/debug/shared/logging/logging.h"
@@ -128,6 +129,11 @@
return 1;
}
+ // The debug agent is independent on whether it's connected or not.
+ // The following loop will attempt to patch a stream to the debug agent in order to enable
+ // communication.
+ debug_agent::DebugAgent debug_agent(services);
+
while (true) {
// Start a new thread that will listen on a socket from an incoming connection from a
// client. In the meantime, the main thread will block waiting for something to be posted
@@ -138,8 +144,11 @@
// the loop, the code will clean the connection thread and create another or exit the loop,
// according to the current agent's configuration.
{
- std::thread conn_thread(&debug_agent::SocketServer::Run, &server, message_loop.get(),
- options.port, services);
+ debug_agent::SocketServer::ConnectionConfig conn_config;
+ conn_config.message_loop = message_loop.get();
+ conn_config.debug_agent = &debug_agent;
+ conn_config.port = options.port;
+ std::thread conn_thread(&debug_agent::SocketServer::Run, &server, std::move(conn_config));
message_loop->Run();
@@ -148,8 +157,7 @@
}
// See if the debug agent was told to exit.
- auto* agent = server.GetDebugAgent();
- if (!agent || agent->should_quit())
+ if (debug_agent.should_quit())
break;
// Prepare for another connection.
diff --git a/src/developer/debug/debug_agent/socket_connection.cc b/src/developer/debug/debug_agent/socket_connection.cc
index 5f69e85..26ef928 100644
--- a/src/developer/debug/debug_agent/socket_connection.cc
+++ b/src/developer/debug/debug_agent/socket_connection.cc
@@ -8,18 +8,13 @@
#include <sys/socket.h>
#include <unistd.h>
+#include "src/developer/debug/debug_agent/debug_agent.h"
#include "src/developer/debug/shared/logging/logging.h"
namespace debug_agent {
// Socket Server -----------------------------------------------------------------------------------
-const DebugAgent* SocketServer::GetDebugAgent() const {
- if (!connected())
- return nullptr;
- return connection_->agent();
-}
-
bool SocketServer::Init(uint16_t port) {
server_socket_.reset(socket(AF_INET6, SOCK_STREAM, IPPROTO_TCP));
if (!server_socket_.is_valid()) {
@@ -44,22 +39,32 @@
return true;
}
-void SocketServer::Run(debug_ipc::MessageLoop* main_thread_loop, int port,
- std::shared_ptr<sys::ServiceDirectory> services) {
+void SocketServer::Run(ConnectionConfig config) {
// Wait for one connection.
- printf("Waiting on port %d for zxdb connection...\n", port);
+ printf("Waiting on port %d for zxdb connection...\n", config.port);
fflush(stdout);
- connection_ = std::make_unique<SocketConnection>(services);
- if (!connection_->Accept(main_thread_loop, server_socket_.get()))
+ connection_ = std::make_unique<SocketConnection>(config.debug_agent);
+ if (!connection_->Accept(config.message_loop, server_socket_.get()))
return;
printf("Connection established.\n");
}
-void SocketServer::Reset() { connection_.reset(); }
+void SocketServer::Reset() {
+ connection_.reset();
+}
// SocketConnection --------------------------------------------------------------------------------
+SocketConnection::SocketConnection(debug_agent::DebugAgent* agent) : debug_agent_(agent) {}
+
+SocketConnection::~SocketConnection() {
+ if (!connected_)
+ return;
+ FXL_DCHECK(debug_agent_) << "A debug agent should be set when resetting the connection.";
+ debug_agent_->Disconnect();
+}
+
bool SocketConnection::Accept(debug_ipc::MessageLoop* main_thread_loop, int server_fd) {
sockaddr_in6 addr;
memset(&addr, 0, sizeof(addr));
@@ -77,7 +82,8 @@
}
// We need to post the agent initialization to the other thread.
- main_thread_loop->PostTask(FROM_HERE, [this, client = std::move(client)]() mutable {
+ main_thread_loop->PostTask(FROM_HERE, [this, debug_agent = debug_agent_,
+ client = std::move(client)]() mutable {
if (!buffer_.Init(std::move(client))) {
FXL_LOG(ERROR) << "Error waiting for data.";
debug_ipc::MessageLoop::Current()->QuitNow();
@@ -85,8 +91,7 @@
}
// Route data from the router_buffer -> RemoteAPIAdapter -> DebugAgent.
- agent_ = std::make_unique<debug_agent::DebugAgent>(&buffer_.stream(), services_);
- adapter_ = std::make_unique<debug_agent::RemoteAPIAdapter>(agent_.get(), &buffer_.stream());
+ adapter_ = std::make_unique<debug_agent::RemoteAPIAdapter>(debug_agent, &buffer_.stream());
buffer_.set_data_available_callback(
[adapter = adapter_.get()]() { adapter->OnStreamReadable(); });
@@ -96,9 +101,13 @@
DEBUG_LOG(Agent) << "Connection lost.";
debug_ipc::MessageLoop::Current()->QuitNow();
});
+
+ // Connect the buffer into the agent.
+ debug_agent->Connect(&buffer_.stream());
});
printf("Accepted connection.\n");
+ connected_ = true;
return true;
}
diff --git a/src/developer/debug/debug_agent/socket_connection.h b/src/developer/debug/debug_agent/socket_connection.h
index 2bd2318..478aa4b 100644
--- a/src/developer/debug/debug_agent/socket_connection.h
+++ b/src/developer/debug/debug_agent/socket_connection.h
@@ -2,12 +2,13 @@
// Use of this source code is governed by a BSD-style license that can be found in the LICENSE file.
#include "lib/sys/cpp/service_directory.h"
-#include "src/developer/debug/debug_agent/debug_agent.h"
#include "src/developer/debug/debug_agent/remote_api_adapter.h"
#include "src/developer/debug/shared/buffered_fd.h"
+#include "src/developer/debug/shared/message_loop_target.h"
namespace debug_agent {
+class DebugAgent;
class SocketConnection;
// SocketServer ------------------------------------------------------------------------------------
@@ -33,12 +34,18 @@
public:
SocketServer() = default;
+ // All fields of |config| must be correctly set.
+ //
// IMPORTANT: Only this can be called on another thread.
// We use |main_thread_loop| to post a task that actually creates the debug agent on
// the main thread after the connection has been made. This is because the agent has a
// lot of assumptions of being run on the thread of the message loop.
- void Run(debug_ipc::MessageLoop* main_thread_loop, int port,
- std::shared_ptr<sys::ServiceDirectory> services);
+ struct ConnectionConfig {
+ debug_ipc::MessageLoopTarget* message_loop = nullptr;
+ debug_agent::DebugAgent* debug_agent = nullptr;
+ int port = 0;
+ };
+ void Run(ConnectionConfig config);
// IMPORTANT: All others can only be called on the main thread.
@@ -47,7 +54,6 @@
void Reset();
bool connected() const { return !!connection_; }
- const DebugAgent* GetDebugAgent() const;
private:
fxl::UniqueFD server_socket_;
@@ -62,22 +68,20 @@
// It owns a DebugAgent, which is currently always associated to one connection.
class SocketConnection {
public:
- SocketConnection(std::shared_ptr<sys::ServiceDirectory> services) : services_(services) {}
- ~SocketConnection() {}
+ SocketConnection(debug_agent::DebugAgent*);
+ ~SocketConnection();
// |main_thread_loop| is used for posting a task that creates the debug agent after accepting a
// a connection. This is because the debug agent assumes it's running on the message loop's
// thread.
bool Accept(debug_ipc::MessageLoop* main_thread_loop, int server_fd);
- const debug_agent::DebugAgent* agent() const { return agent_.get(); }
-
private:
- std::shared_ptr<sys::ServiceDirectory> services_;
- debug_ipc::BufferedFD buffer_;
+ debug_agent::DebugAgent* debug_agent_ = nullptr;
- std::unique_ptr<debug_agent::DebugAgent> agent_;
std::unique_ptr<debug_agent::RemoteAPIAdapter> adapter_;
+ debug_ipc::BufferedFD buffer_;
+ bool connected_ = false;
FXL_DISALLOW_COPY_AND_ASSIGN(SocketConnection);
};
diff --git a/src/developer/debug/zxdb/client/system.cc b/src/developer/debug/zxdb/client/system.cc
index 01c6e44..c14a88b 100644
--- a/src/developer/debug/zxdb/client/system.cc
+++ b/src/developer/debug/zxdb/client/system.cc
@@ -63,7 +63,7 @@
schema->AddList(ClientSettings::System::kSymbolPaths, kSymbolPathsDescription, {});
schema->AddBool(ClientSettings::System::kPauseOnLaunch, kPauseOnLaunchDescription, false);
schema->AddBool(ClientSettings::System::kPauseOnAttach, kPauseOnAttachDescription, false);
- schema->AddBool(ClientSettings::System::kQuitAgentOnExit, kQuitAgentOnExitDescription, false);
+ schema->AddBool(ClientSettings::System::kQuitAgentOnExit, kQuitAgentOnExitDescription, true);
schema->AddBool(ClientSettings::System::kShowStdout, kShowStdoutDescription, true);
schema->AddList(ClientSettings::System::kSymbolServers, kSymbolServersDescription, {});
schema->AddString(ClientSettings::System::kSymbolCache, kSymbolCacheDescription, "");
