diff --git a/src/Application.cpp b/src/Application.cpp index 0dec377f..2b0ae3dd 100644 --- a/src/Application.cpp +++ b/src/Application.cpp @@ -517,6 +517,7 @@ void Application::stopTargetController() { void Application::startDebugServer() { this->debugServer = std::make_unique( + this->environmentConfig.value(), this->debugServerConfig.value() ); this->debugServerThread = std::thread{&DebugServer::DebugServerComponent::run, this->debugServer.get()}; @@ -637,7 +638,10 @@ void Application::onDebugServerThreadStateChanged(const Events::DebugServerThrea } void Application::onDebugSessionFinished(const Events::DebugSessionFinished& event) { - if (this->environmentConfig->shutdownPostDebugSession || Services::ProcessService::isManagedByClion()) { + if ( + this->environmentConfig->shutdownPostDebugSession + || (this->environmentConfig->clionAdaptation && Services::ProcessService::isManagedByClion()) + ) { this->triggerShutdown(); } } diff --git a/src/DebugServer/DebugServerComponent.cpp b/src/DebugServer/DebugServerComponent.cpp index c884e5f9..71ab3886 100644 --- a/src/DebugServer/DebugServerComponent.cpp +++ b/src/DebugServer/DebugServerComponent.cpp @@ -13,8 +13,12 @@ namespace DebugServer { using namespace Events; - DebugServerComponent::DebugServerComponent(const DebugServerConfig& debugServerConfig) - : debugServerConfig(debugServerConfig) + DebugServerComponent::DebugServerComponent( + const EnvironmentConfig& environmentConfig, + const DebugServerConfig& debugServerConfig + ) + : environmentConfig(environmentConfig) + , debugServerConfig(debugServerConfig) , targetDescriptor((Services::TargetControllerService()).getTargetDescriptor()) {} @@ -49,6 +53,7 @@ namespace DebugServer } return std::make_unique( + this->environmentConfig, this->debugServerConfig, this->targetDescriptor, *(this->eventListener.get()), @@ -64,6 +69,7 @@ namespace DebugServer } return std::make_unique( + this->environmentConfig, this->debugServerConfig, this->targetDescriptor, *(this->eventListener.get()), diff --git a/src/DebugServer/DebugServerComponent.hpp b/src/DebugServer/DebugServerComponent.hpp index cde4fe6f..1803ccd1 100644 --- a/src/DebugServer/DebugServerComponent.hpp +++ b/src/DebugServer/DebugServerComponent.hpp @@ -25,7 +25,10 @@ namespace DebugServer class DebugServerComponent: public Thread { public: - explicit DebugServerComponent(const DebugServerConfig& debugServerConfig); + explicit DebugServerComponent( + const EnvironmentConfig& environmentConfig, + const DebugServerConfig& debugServerConfig + ); /** * Entry point for the DebugServer. This must called from a dedicated thread. @@ -44,9 +47,7 @@ namespace DebugServer */ EventListenerPointer eventListener = std::make_shared("DebugServerEventListener"); - /** - * Project configuration for the debug server (extracted from the user project's bloom.yaml). - */ + EnvironmentConfig environmentConfig; DebugServerConfig debugServerConfig; /** diff --git a/src/DebugServer/Gdb/AvrGdb/AvrGdbRsp.cpp b/src/DebugServer/Gdb/AvrGdb/AvrGdbRsp.cpp index 14696622..91a1039e 100644 --- a/src/DebugServer/Gdb/AvrGdb/AvrGdbRsp.cpp +++ b/src/DebugServer/Gdb/AvrGdb/AvrGdbRsp.cpp @@ -29,12 +29,14 @@ namespace DebugServer::Gdb::AvrGdb using Targets::TargetRegisterType; AvrGdbRsp::AvrGdbRsp( + const EnvironmentConfig& environmentConfig, const DebugServerConfig& debugServerConfig, const Targets::TargetDescriptor& targetDescriptor, EventListener& eventListener, EventFdNotifier& eventNotifier ) : GdbRspDebugServer( + environmentConfig, debugServerConfig, targetDescriptor, AvrGdbTargetDescriptor{targetDescriptor}, diff --git a/src/DebugServer/Gdb/AvrGdb/AvrGdbRsp.hpp b/src/DebugServer/Gdb/AvrGdb/AvrGdbRsp.hpp index 5bb3675f..9d73bdb0 100644 --- a/src/DebugServer/Gdb/AvrGdb/AvrGdbRsp.hpp +++ b/src/DebugServer/Gdb/AvrGdb/AvrGdbRsp.hpp @@ -14,6 +14,7 @@ namespace DebugServer::Gdb::AvrGdb { public: AvrGdbRsp( + const EnvironmentConfig& environmentConfig, const DebugServerConfig& debugServerConfig, const Targets::TargetDescriptor& targetDescriptor, EventListener& eventListener, diff --git a/src/DebugServer/Gdb/CommandPackets/Detach.cpp b/src/DebugServer/Gdb/CommandPackets/Detach.cpp index 01d798d7..3a657528 100644 --- a/src/DebugServer/Gdb/CommandPackets/Detach.cpp +++ b/src/DebugServer/Gdb/CommandPackets/Detach.cpp @@ -16,8 +16,9 @@ namespace DebugServer::Gdb::CommandPackets using ::Exceptions::Exception; - Detach::Detach(const RawPacket& rawPacket) + Detach::Detach(const RawPacket& rawPacket, const EnvironmentConfig& environmentConfig) : CommandPacket(rawPacket) + , environmentConfig(environmentConfig) {} void Detach::handle( @@ -29,7 +30,14 @@ namespace DebugServer::Gdb::CommandPackets Logger::info("Handling Detach packet"); try { - if (Services::ProcessService::isManagedByClion()) { + if (this->environmentConfig.clionAdaptation && Services::ProcessService::isManagedByClion()) { + /* + * CLion is about to kill Bloom's process. This is our last chance to detach from the target and + * disconnect from the debug tool, cleanly. + * + * So we have to force the TC to shut down immediately, to prevent CLion from killing Bloom and leaving + * the debug tool and target in an undefined state. + */ targetControllerService.shutdown(); } diff --git a/src/DebugServer/Gdb/CommandPackets/Detach.hpp b/src/DebugServer/Gdb/CommandPackets/Detach.hpp index 39da1199..cc963296 100644 --- a/src/DebugServer/Gdb/CommandPackets/Detach.hpp +++ b/src/DebugServer/Gdb/CommandPackets/Detach.hpp @@ -2,12 +2,14 @@ #include "CommandPacket.hpp" +#include "src/ProjectConfig.hpp" + namespace DebugServer::Gdb::CommandPackets { class Detach: public CommandPacket { public: - explicit Detach(const RawPacket& rawPacket); + explicit Detach(const RawPacket& rawPacket, const EnvironmentConfig& environmentConfig); void handle( DebugSession& debugSession, @@ -15,5 +17,8 @@ namespace DebugServer::Gdb::CommandPackets const Targets::TargetDescriptor& targetDescriptor, Services::TargetControllerService& targetControllerService ) override; + + private: + const EnvironmentConfig& environmentConfig; }; } diff --git a/src/DebugServer/Gdb/GdbRspDebugServer.hpp b/src/DebugServer/Gdb/GdbRspDebugServer.hpp index c9e144b4..bca435f8 100644 --- a/src/DebugServer/Gdb/GdbRspDebugServer.hpp +++ b/src/DebugServer/Gdb/GdbRspDebugServer.hpp @@ -86,13 +86,15 @@ namespace DebugServer::Gdb { public: explicit GdbRspDebugServer( + const EnvironmentConfig& environmentConfig, const DebugServerConfig& debugServerConfig, const Targets::TargetDescriptor& targetDescriptor, GdbTargetDescriptorType&& gdbTargetDescriptor, EventListener& eventListener, EventFdNotifier& eventNotifier ) - : debugServerConfig(GdbDebugServerConfig{debugServerConfig}) + : environmentConfig(environmentConfig) + , debugServerConfig(GdbDebugServerConfig{debugServerConfig}) , targetDescriptor(targetDescriptor) , gdbTargetDescriptor(std::move(gdbTargetDescriptor)) , eventListener(eventListener) @@ -179,9 +181,10 @@ namespace DebugServer::Gdb std::bind(&GdbRspDebugServer::onTargetStateChanged, this, std::placeholders::_1) ); - if (Services::ProcessService::isManagedByClion()) { + if (this->environmentConfig.clionAdaptation && Services::ProcessService::isManagedByClion()) { Logger::warning( - "Bloom's process is being managed by CLion - Bloom will automatically shutdown upon detaching from GDB." + "Bloom's process is being managed by CLion - Bloom will automatically shut down upon" + " detaching from GDB." ); } } @@ -286,6 +289,7 @@ namespace DebugServer::Gdb } protected: + const EnvironmentConfig& environmentConfig; GdbDebugServerConfig debugServerConfig; const Targets::TargetDescriptor& targetDescriptor; GdbTargetDescriptorType gdbTargetDescriptor; @@ -434,7 +438,7 @@ namespace DebugServer::Gdb } if (rawPacket[1] == 'D') { - return std::make_unique(rawPacket); + return std::make_unique(rawPacket, this->environmentConfig); } const auto rawPacketString = std::string{rawPacket.begin() + 1, rawPacket.end()}; diff --git a/src/DebugServer/Gdb/RiscVGdb/RiscVGdbRsp.cpp b/src/DebugServer/Gdb/RiscVGdb/RiscVGdbRsp.cpp index 955cc5d4..d149827d 100644 --- a/src/DebugServer/Gdb/RiscVGdb/RiscVGdbRsp.cpp +++ b/src/DebugServer/Gdb/RiscVGdb/RiscVGdbRsp.cpp @@ -26,12 +26,14 @@ namespace DebugServer::Gdb::RiscVGdb using Targets::TargetRegisterType; RiscVGdbRsp::RiscVGdbRsp( + const EnvironmentConfig& environmentConfig, const DebugServerConfig& debugServerConfig, const Targets::TargetDescriptor& targetDescriptor, EventListener& eventListener, EventFdNotifier& eventNotifier ) : GdbRspDebugServer( + environmentConfig, debugServerConfig, targetDescriptor, RiscVGdbTargetDescriptor{targetDescriptor}, diff --git a/src/DebugServer/Gdb/RiscVGdb/RiscVGdbRsp.hpp b/src/DebugServer/Gdb/RiscVGdb/RiscVGdbRsp.hpp index 717e8701..cf0d2766 100644 --- a/src/DebugServer/Gdb/RiscVGdb/RiscVGdbRsp.hpp +++ b/src/DebugServer/Gdb/RiscVGdb/RiscVGdbRsp.hpp @@ -17,6 +17,7 @@ namespace DebugServer::Gdb::RiscVGdb { public: RiscVGdbRsp( + const EnvironmentConfig& environmentConfig, const DebugServerConfig& debugServerConfig, const Targets::TargetDescriptor& targetDescriptor, EventListener& eventListener, diff --git a/src/ProjectConfig.cpp b/src/ProjectConfig.cpp index 5d2493ab..b91cab0f 100644 --- a/src/ProjectConfig.cpp +++ b/src/ProjectConfig.cpp @@ -152,6 +152,10 @@ EnvironmentConfig::EnvironmentConfig(std::string name, const YAML::Node& environ this->shutdownPostDebugSession ); } + + if (environmentNode["clion_adaptation"]) { + this->clionAdaptation = environmentNode["clion_adaptation"].as(this->clionAdaptation); + } } TargetConfig::TargetConfig(const YAML::Node& targetNode) { diff --git a/src/ProjectConfig.hpp b/src/ProjectConfig.hpp index aa54cd7a..5899848e 100644 --- a/src/ProjectConfig.hpp +++ b/src/ProjectConfig.hpp @@ -30,57 +30,17 @@ */ struct TargetConfig { - /** - * The name of the selected target. - */ std::string name; - - /** - * The physical interface is the interface used for communication between the debug tool and the connected - * target. - */ Targets::TargetPhysicalInterface physicalInterface; - - /** - * Determines whether Bloom will resume target execution after activation. - */ bool resumeOnStartup = false; - - /** - * Determines whether Bloom will make use of the target's hardware breakpoint resources (if available). - */ bool hardwareBreakpoints = true; - - /** - * Determines whether Bloom will employ a cache for the target's program memory. - */ bool programMemoryCache = true; - - /** - * Determines whether Bloom will employ "delta programming" during programming sessions. - * - * Not all targets support delta programming. - */ bool deltaProgramming = true; - - /** - * Determines if Bloom will reserve a single hardware breakpoint for stepping operations. - */ std::optional reserveSteppingBreakpoint = std::nullopt; - /** - * For extracting any target specific configuration. See Avr8TargetConfig::Avr8TargetConfig() for an example of - * this. - */ YAML::Node targetNode; TargetConfig() = default; - - /** - * Obtains config parameters from YAML node. - * - * @param targetNode - */ explicit TargetConfig(const YAML::Node& targetNode); }; @@ -93,23 +53,11 @@ struct TargetConfig */ struct DebugToolConfig { - /** - * The name of the selected debug tool. - */ std::string name; - /** - * For extracting any debug tool specific configuration. - */ YAML::Node toolNode; DebugToolConfig() = default; - - /** - * Obtains config parameters from YAML node. - * - * @param toolNode - */ explicit DebugToolConfig(const YAML::Node& toolNode); }; @@ -118,51 +66,21 @@ struct DebugToolConfig */ struct DebugServerConfig { - /** - * The name of the selected debug server. - */ std::string name; - /** - * For extracting any debug server specific configuration. See GdbDebugServerConfig::GdbDebugServerConfig() and - * GdbRspDebugServer::GdbRspDebugServer() for an example of this. - */ YAML::Node debugServerNode; DebugServerConfig() = default; - - /** - * Obtains config parameters from YAML node. - * - * @param debugServerNode - */ explicit DebugServerConfig(const YAML::Node& debugServerNode); }; struct InsightConfig { - /** - * If true, the Insight GUI will be activated immediately at startup. - */ bool activateOnStartup = false; - - /** - * If true, Bloom will shutdown when the user closes the Insight GUI. - */ bool shutdownOnClose = false; - - /** - * The key of the variant to select by default, in the Insight GUi. - */ - std::optional defaultVariantKey; + std::optional defaultVariantKey = std::nullopt; InsightConfig() = default; - - /** - * Obtains config parameters from YAML node. - * - * @param insightNode - */ explicit InsightConfig(const YAML::Node& insightNode); }; @@ -174,49 +92,33 @@ struct InsightConfig */ struct EnvironmentConfig { - /** - * The environment name is stored as the key to the YAML map containing the environment parameters. - * - * Environment names must be unique. - */ std::string name; - - /** - * Flag to determine whether Bloom should shutdown at the end of a debug session. - */ bool shutdownPostDebugSession = false; /** - * Configuration for the environment's selected debug tool. + * CLion expects the debug server (in this case, Bloom) to shut down immediately upon receiving the detach command + * via GDB. It does not allow for any time to shut down after that - it just kills the process with a SIGKILL. * - * Each environment can select only one debug tool. + * I raised this with JetBrains, a number of years ago: https://youtrack.jetbrains.com/issue/CPP-28843 + * + * They didn't seem to have any interest in fixing the issue, so I had to include some CLion-specific behaviour + * in Bloom, to prevent CLion from killing Bloom's process before we got a chance to shut down cleanly. It was a + * hacky solution, but it worked. + * + * Recently (in version 2024.3), CLion introduced "debug servers", which allows Bloom to remain running in the + * background, between debug sessions, reducing the time it takes to stop and start new debug sessions. + * + * This doesn't fix the issue described above - CLion still behaves like an aggressive dickhead, sending SIGKILL + * signals whenever it likes. However, Bloom's CLion-specific functionality may conflict with this new "debug server" + * functionality, so it's necessary to allow the user to disable it in their project config. */ + bool clionAdaptation = true; + DebugToolConfig debugToolConfig; - - /** - * Configuration for the environment's selected target. - * - * Each environment can select only one target. - */ TargetConfig targetConfig; - - /** - * Configuration for the environment's debug server. Users can define this at the application level if - * they desire. - */ std::optional debugServerConfig; - - /** - * Insight configuration can be defined at an environment level as well as at an application level. - */ std::optional insightConfig; - /** - * Obtains config parameters from YAML node. - * - * @param name - * @param environmentNode - */ EnvironmentConfig(std::string name, const YAML::Node& environmentNode); }; @@ -225,33 +127,10 @@ struct EnvironmentConfig */ struct ProjectConfig { - /** - * A mapping of environment names to EnvironmentConfig objects. - */ std::map environments; - - /** - * Application level debug server configuration. We use this as a fallback if no debug server config is - * provided at the environment level. - */ std::optional debugServerConfig; - - /** - * Application level Insight configuration. We use this as a fallback if no Insight config is provided at - * the environment level. - * - * We don't use std::optional here because the InsightConfig has no mandatory parameters, so users may wish to - * omit the 'insight' node from their bloom.yaml file, entirely. In this case, Bloom should fall back to a default - * constructed, project-level, InsightConfig instance. - */ InsightConfig insightConfig = {}; - bool debugLogging = false; - /** - * Obtains config parameters from YAML node. - * - * @param configNode - */ explicit ProjectConfig(const YAML::Node& configNode); };