Merge pull request #491 from wazuh/enhancement/488-agent-lock-file

Implements RAII for the creation and deletion of the wazuh-agent.lock file
wazuh · Jan 10, 2025 · ec649d1 · ec649d1
2 parents f5f4eed + b7a3076
commit ec649d1
Show file tree

Hide file tree

Showing 4 changed files with 52 additions and 21 deletions.
diff --git a/src/agent/src/process_options_unix.cpp b/src/agent/src/process_options_unix.cpp
@@ -32,11 +32,9 @@ void StartAgent(const std::string& configFilePath)
     {
         LogError("Exception thrown in wazuh-agent: {}", e.what());
     }
-
-    lockFileHandler.removeLockFile();
 }
 
 void StatusAgent(const std::string& configFilePath)
 {
-    std::cout << fmt::format("wazuh-agent is {}\n", unix_daemon::GetDaemonStatus(configFilePath));
+    std::cout << fmt::format("wazuh-agent status: {}\n", unix_daemon::GetDaemonStatus(configFilePath));
 }
diff --git a/src/agent/src/unix/unix_daemon.cpp b/src/agent/src/unix/unix_daemon.cpp
@@ -6,8 +6,10 @@
 
 #include <cerrno>
 #include <filesystem>
+#include <fmt/format.h>
 #include <fstream>
 #include <sys/file.h>
+#include <unistd.h>
 
 #if defined(__linux__)
 #include <unistd.h>
@@ -23,22 +25,24 @@ namespace unix_daemon
 {
     LockFileHandler::LockFileHandler(std::string lockFilePath)
         : m_lockFilePath(std::move(lockFilePath))
+        , m_errno(0)
         , m_lockFileCreated(createLockFile())
     {
     }
 
-    bool LockFileHandler::removeLockFile() const
+    void LockFileHandler::removeLockFile() const
     {
+        if (!m_lockFileCreated)
+            return;
+
         const std::string filePath = fmt::format("{}/wazuh-agent.lock", m_lockFilePath);
         try
         {
             std::filesystem::remove(filePath);
-            return true;
         }
         catch (const std::filesystem::filesystem_error& e)
         {
             LogError("Error removing file: {}", e.what());
-            return false;
         }
     }
 
@@ -75,13 +79,15 @@ namespace unix_daemon
         int fd = open(filename.c_str(), O_WRONLY | O_CREAT | O_TRUNC, 0644);
         if (fd == -1)
         {
-            LogError("Unable to open lock file: {}. Error: {} ({})", filename.c_str(), errno, std::strerror(errno));
+            m_errno = errno;
+            LogError("Unable to open lock file: {}. Error: {} ({})", filename.c_str(), m_errno, std::strerror(m_errno));
             return false;
         }
 
         if (flock(fd, LOCK_EX | LOCK_NB) == -1)
         {
-            LogDebug("Unable to lock lock file: {}. Error: {} ({})", filename.c_str(), errno, std::strerror(errno));
+            m_errno = errno;
+            LogDebug("Unable to lock lock file: {}. Error: {} ({})", filename.c_str(), m_errno, std::strerror(m_errno));
             close(fd);
             return false;
         }
@@ -109,10 +115,17 @@ namespace unix_daemon
 
         if (!lockFileHandler.isLockFileCreated())
         {
-            return "running";
+            if (lockFileHandler.getErrno() == EAGAIN)
+            {
+                return "running";
+            }
+            else
+            {
+                return fmt::format(
+                    "Error: {} ({})", lockFileHandler.getErrno(), std::strerror(lockFileHandler.getErrno()));
+            }
         }
 
-        lockFileHandler.removeLockFile();
         return "stopped";
     }
 } // namespace unix_daemon
diff --git a/src/agent/src/unix/unix_daemon.hpp b/src/agent/src/unix/unix_daemon.hpp
@@ -1,8 +1,6 @@
 #pragma once
 
 #include <string>
-#include <unistd.h>
-#include <vector>
 
 namespace unix_daemon
 {
@@ -16,16 +14,26 @@ namespace unix_daemon
         /// @param lockFilePath The path where the lock file will be created
         LockFileHandler(std::string lockFilePath);
 
+        /// @brief Destructor
+        /// @details Removes lock file if it was created on construction
+        ~LockFileHandler()
+        {
+            removeLockFile();
+        }
+
         /// @brief Checks if the lock file has been successfully created
         /// @return True if the lock file is created, false otherwise
         bool isLockFileCreated() const
         {
             return m_lockFileCreated;
         }
 
-        /// @brief Removes the lock file
-        /// @return True if the file is removed, false otherwise
-        bool removeLockFile() const;
+        /// @brief Returns the errno from the latest attempt to create/lock the lock-file
+        /// @return The errno
+        int getErrno() const
+        {
+            return m_errno;
+        }
 
     private:
         /// @brief Creates the directory path for the lock file
@@ -37,7 +45,15 @@ namespace unix_daemon
         /// @return True if the lock file is created, false otherwise
         bool createLockFile();
 
+        /// @brief Removes the lock file
+        void removeLockFile() const;
+
         std::string m_lockFilePath;
+
+        /// @brief Holds the errno from the lock-file create/lock attempt
+        int m_errno;
+
+        /// @brief Indicates the lock file was created by this instance of the LockFileHandler
         bool m_lockFileCreated;
     };
 

diff --git a/src/agent/tests/unix/unix_daemon_test.cpp b/src/agent/tests/unix/unix_daemon_test.cpp
@@ -35,7 +35,6 @@ TEST_F(UnixDaemonTest, CreateLockFile)
     unix_daemon::LockFileHandler lockFileHandler = unix_daemon::GenerateLockFile("./temp_wazuh-agent.yml");
     bool res = lockFileHandler.isLockFileCreated();
     ASSERT_TRUE(res);
-    lockFileHandler.removeLockFile();
 }
 
 TEST_F(UnixDaemonTest, CreateLockFileTwice)
@@ -48,21 +47,26 @@ TEST_F(UnixDaemonTest, CreateLockFileTwice)
 
     ASSERT_TRUE(reslockFileHandler);
     ASSERT_FALSE(reslockFileHandler2);
-    lockFileHandler.removeLockFile();
 }
 
 TEST_F(UnixDaemonTest, GetDaemonStatusRunning)
 {
     unix_daemon::LockFileHandler lockFileHandler = unix_daemon::GenerateLockFile("./temp_wazuh-agent.yml");
     std::string res = unix_daemon::GetDaemonStatus("./temp_wazuh-agent.yml");
     ASSERT_EQ(res, "running");
-    lockFileHandler.removeLockFile();
 }
 
-TEST_F(UnixDaemonTest, GetDaemonStatusStopped)
+TEST_F(UnixDaemonTest, GetDaemonStatusStoppedPrevInstanceDestroyed)
+{
+    {
+        unix_daemon::LockFileHandler lockFileHandler = unix_daemon::GenerateLockFile("./temp_wazuh-agent.yml");
+    }
+    std::string res = unix_daemon::GetDaemonStatus("./temp_wazuh-agent.yml");
+    ASSERT_EQ(res, "stopped");
+}
+
+TEST_F(UnixDaemonTest, GetDaemonStatusStoppedNoPrevInstance)
 {
-    unix_daemon::LockFileHandler lockFileHandler = unix_daemon::GenerateLockFile("./temp_wazuh-agent.yml");
-    lockFileHandler.removeLockFile();
     std::string res = unix_daemon::GetDaemonStatus("./temp_wazuh-agent.yml");
     ASSERT_EQ(res, "stopped");
 }