New clion_adaptation environment config param, to allow for use of Bloom with CLion's new "debug server" functionality

src/Application.cpp

@@ -517,6 +517,7 @@ void Application::stopTargetController() {
 
 void Application::startDebugServer() {
     this->debugServer = std::make_unique<DebugServer::DebugServerComponent>(
+        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();
     }
 }

src/DebugServer/DebugServerComponent.cpp

@@ -13,8 +13,12 @@ namespace DebugServer
 {
     using namespace Events;
 
-    DebugServerComponent::DebugServerComponent(const DebugServerConfig& debugServerConfig)
+    DebugServerComponent::DebugServerComponent(
-        : debugServerConfig(debugServerConfig)
+        const EnvironmentConfig& environmentConfig,
+        const DebugServerConfig& debugServerConfig
+    )
+        : environmentConfig(environmentConfig)
+        , debugServerConfig(debugServerConfig)
         , targetDescriptor((Services::TargetControllerService()).getTargetDescriptor())
     {}
 
@@ -49,6 +53,7 @@ namespace DebugServer
                     }
 
                     return std::make_unique<DebugServer::Gdb::AvrGdb::AvrGdbRsp>(
+                        this->environmentConfig,
                         this->debugServerConfig,
                         this->targetDescriptor,
                         *(this->eventListener.get()),
@@ -64,6 +69,7 @@ namespace DebugServer
                     }
 
                     return std::make_unique<DebugServer::Gdb::RiscVGdb::RiscVGdbRsp>(
+                        this->environmentConfig,
                         this->debugServerConfig,
                         this->targetDescriptor,
                         *(this->eventListener.get()),

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<EventListener>("DebugServerEventListener");
 
-        /**
+        EnvironmentConfig environmentConfig;
-         * Project configuration for the debug server (extracted from the user project's bloom.yaml).
-         */
         DebugServerConfig debugServerConfig;
 
         /**

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},

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,

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();
             }
 

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;
     };
 }

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<CommandPackets::Detach>(rawPacket);
+                return std::make_unique<CommandPackets::Detach>(rawPacket, this->environmentConfig);
             }
 
             const auto rawPacketString = std::string{rawPacket.begin() + 1, rawPacket.end()};

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},

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,

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<bool>(this->clionAdaptation);
+    }
 }
 
 TargetConfig::TargetConfig(const YAML::Node& targetNode) {

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<bool> 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;
-
+    std::optional<std::string> defaultVariantKey = std::nullopt;
-    /**
-     * The key of the variant to select by default, in the Insight GUi.
-     */
-    std::optional<std::string> defaultVariantKey;
 
     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> debugServerConfig;
-
-    /**
-     * Insight configuration can be defined at an environment level as well as at an application level.
-     */
     std::optional<InsightConfig> 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<std::string, EnvironmentConfig> 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> 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);
 };