Update lockfile handler

integration-tests/entry-point.sh

@@ -8,9 +8,6 @@ CERT_PATH=${CERT_DIRECTORY}cert.crt
 KEY_PATH=${CERT_DIRECTORY}key.pem
 ROOT_CA_PATH=${CERT_DIRECTORY}AmazonRootCA1.pem
 
-# Ensure /run/lock exists
-mkdir -p /run/lock > dev/null
-
 # create Certificate Directory
 mkdir -p ${CERT_DIRECTORY}
 
@@ -60,6 +57,36 @@ chmod 640 ${CONFIG_PATH}
 # start ssh
 service ssh start || true
 
+# Ensure lockfile exists
+mkdir -p /run/lock > dev/null
+
+# Spoof lockfile
+echo "${THING_NAME}" > /run/lock/devicecl.lock
+bash -c 'sleep 1000' &
+FAKE_PID=$!
+echo ${FAKE_PID} >> /run/lock/devicecl.lock
+
+# TEST: Start and stop Device Client
+# 1. Rename aws-iot-device-client to sleep and exec binary
+# 2. Kill dummy process
+# 3. Kill Device Client. If Device Client has already exited as expected, then pkill will return 1, and we pass the test
+cp aws-iot-device-client sleep
+./sleep &
+DC_PID=$!
+
+# give some buffer time for the background instance of DC to run its logic.
+sleep 5
+kill $FAKE_PID
+kill $DC_PID
+retVal=$?
+if [ $retVal -ne 1 ]; then
+  echo 'TEST FAILURE: Device Client ran with a valid lockfile already in place.'
+  exit 1
+fi
+
+# Cleanup
+rm -rf /run/lock
+
 # start Device Client
 ./aws-iot-device-client 2>&1 &
 

source/config/Config.cpp

@@ -326,15 +326,6 @@ bool PlainConfig::Validate() const
     {
         return false;
     }
-    if (lockFilePath.empty() || !FileUtils::IsValidFilePath(lockFilePath))
-    {
-        LOGM_ERROR(
-            Config::TAG,
-            "*** %s: Invalid Lock File Path %s ***",
-            DeviceClient::DC_FATAL_ERROR,
-            Sanitize(lockFilePath).c_str());
-        return false;
-    }
     if (rootCa.has_value() && !rootCa->empty() && FileUtils::FileExists(rootCa->c_str()))
     {
         string parentDir = FileUtils::ExtractParentDirectory(rootCa->c_str());

source/main.cpp

@@ -113,7 +113,13 @@ bool init(int argc, char *argv[])
         string filename = config.config.lockFilePath;
         if (!filename.empty())
         {
-            lockFile = unique_ptr<LockFile>(new LockFile{filename, argv[0]});
+            string thing;
+            if (config.config.thingName.has_value() && !config.config.thingName.value().empty())
+            {
+                thing = config.config.thingName.value();
+            }
+
+            lockFile = unique_ptr<LockFile>(new LockFile{filename, argv[0], thing});
         }
     }
     catch (std::runtime_error &e)
@@ -316,7 +322,7 @@ int main(int argc, char *argv[])
      */
     if (!init(argc, argv))
     {
-        return -1;
+        return 1;
     }
     features = make_shared<FeatureRegistry>();
 
@@ -537,6 +543,9 @@ int main(int argc, char *argv[])
             case SIGINT:
                 shutdown();
                 break;
+            case SIGTERM:
+                shutdown();
+                break;
             case SIGHUP:
                 resourceManager->dumpMemTrace();
                 break;

source/util/LockFile.cpp

@@ -16,21 +16,22 @@ using namespace Aws::Iot::DeviceClient::Logging;
 constexpr char LockFile::TAG[];
 constexpr char LockFile::FILE_NAME[];
 
-LockFile::LockFile(const std::string &filedir, const std::string &process) : dir(filedir)
+LockFile::LockFile(const std::string &filedir, const std::string &process, const std::string &thingName) : dir(filedir)
 {
+    LOG_DEBUG(TAG, "creating lockfile");
     string fullPath = dir + FILE_NAME;
     ifstream fileIn(fullPath);
     if (fileIn)
     {
+        string storedThingName;
         string storedPid;
-        if (fileIn >> storedPid && !(kill(stoi(storedPid), 0) == -1 && errno == ESRCH))
+        if (fileIn >> storedThingName && storedThingName == thingName && fileIn >> storedPid &&
+            !(kill(stoi(storedPid), 0) == -1 && errno == ESRCH))
         {
             string processPath = "/proc/" + storedPid + "/cmdline";
             string basename = process.substr(process.find_last_of("/\\") + 1);
             string cmdline;
             ifstream cmd(processPath.c_str());
-
-            // check if process contains name
             if (cmd && cmd >> cmdline && cmdline.find(basename) != string::npos)
             {
                 LOGM_ERROR(
@@ -56,14 +57,17 @@ LockFile::LockFile(const std::string &filedir, const std::string &process) : dir
     FILE *file = fopen(fullPath.c_str(), "wx");
     if (!file)
     {
-        LOGM_ERROR(TAG, "Unable to open lockfile: %s", Sanitize(fullPath).c_str());
-
-        throw runtime_error{"Can not write to lockfile."};
+        LOGM_ERROR(
+            TAG, "Unable to open lockfile. File may be in use or does not exist: %s", Sanitize(fullPath).c_str());
+    }
+    else
+    {
+        string pid = to_string(getpid());
+        fputs(thingName.c_str(), file);
+        fputs(string("\n").c_str(), file);
+        fputs(pid.c_str(), file);
+        fclose(file);
     }
-
-    string pid = to_string(getpid());
-    fputs(pid.c_str(), file);
-    fclose(file);
 }
 
 LockFile::~LockFile()

source/util/LockFile.h

@@ -31,7 +31,7 @@ namespace Aws
                      * @param filedir directory the lockfile will be written to
                      * @param process the executable path passed in by argv[0], usually aws-iot-device-client
                      */
-                    explicit LockFile(const std::string &filedir, const std::string &process);
+                    LockFile(const std::string &filedir, const std::string &process, const std::string &thingName);
                     /**
                      * This class uses RAII for resource management. Destructor will be called on program exit and the
                      * file will be deleted.

test/util/TestLockFile.cpp

@@ -14,69 +14,81 @@ using namespace Aws::Iot::DeviceClient::Util;
 TEST(LockFile, normalCreation)
 {
     string path = "/run/lock/";
-    string filename = "devicecl.lock";
-    unique_ptr<LockFile> lockFile = unique_ptr<LockFile>(new LockFile{path, "./aws-iot-device-client"});
+    string fileName = "devicecl.lock";
+    string thingName = "thing";
+    unique_ptr<LockFile> lockFile = unique_ptr<LockFile>(new LockFile{path, "./aws-iot-device-client", thingName});
 
-    ifstream fileIn(path + filename);
+    ifstream fileIn(path + fileName);
     ASSERT_TRUE(fileIn);
 
     string storedPid;
-    if (fileIn >> storedPid)
+    string storedName;
+    if (fileIn >> storedName && fileIn >> storedPid)
     {
+        ASSERT_STREQ(thingName.c_str(), storedName.c_str());
         ASSERT_STREQ(to_string(getpid()).c_str(), storedPid.c_str());
     }
 }
 
 TEST(LockFile, earlyDeletion)
 {
     string path = "/run/lock/";
-    string filename = "devicecl.lock";
-    unique_ptr<LockFile> lockFile = unique_ptr<LockFile>(new LockFile{path, "test-aws-iot-device-client"});
+    string fileName = "devicecl.lock";
+    string thingName = "thing";
+    unique_ptr<LockFile> lockFile = unique_ptr<LockFile>(new LockFile{path, "test-aws-iot-device-client", thingName});
     lockFile.reset();
 
-    ifstream fileIn(path + filename);
+    ifstream fileIn(path + fileName);
     ASSERT_FALSE(fileIn);
 }
 
 TEST(LockFile, multipleFiles)
 {
     string path = "/run/lock/";
-    unique_ptr<LockFile> lockFile = unique_ptr<LockFile>(new LockFile{path, "test-aws-iot-device-client"});
+    string thingName = "thing";
+    unique_ptr<LockFile> lockFile = unique_ptr<LockFile>(new LockFile{path, "test-aws-iot-device-client", thingName});
 
-    EXPECT_THROW(unique_ptr<LockFile>(new LockFile{path, "test-aws-iot-device-client"}), std::runtime_error);
+    EXPECT_THROW(unique_ptr<LockFile>(new LockFile{path, "test-aws-iot-device-client", thingName}), std::runtime_error);
 }
 
 TEST(LockFile, multipleFilesWithExtendedPath)
 {
     string path = "/run/lock/";
-    unique_ptr<LockFile> lockFile = unique_ptr<LockFile>(new LockFile{path, "test-aws-iot-device-client"});
+    string thingName = "thing";
+    unique_ptr<LockFile> lockFile = unique_ptr<LockFile>(new LockFile{path, "test-aws-iot-device-client", thingName});
 
-    EXPECT_THROW(unique_ptr<LockFile>(new LockFile{path, "directory/test-aws-iot-device-client"}), std::runtime_error);
+    EXPECT_THROW(
+        unique_ptr<LockFile>(new LockFile{path, "directory/test-aws-iot-device-client", thingName}),
+        std::runtime_error);
 }
 
 TEST(LockFile, staleFile)
 {
     string path = "/run/lock/";
-    string filename = "devicecl.lock";
+    string fileName = "devicecl.lock";
+    string thingName = "thing";
     string pidMax;
     ifstream pidFile("/proc/sys/kernel/pid_max");
     if (pidFile && pidFile >> pidMax)
     {
-        ofstream fileOut(path + filename);
+        ofstream fileOut(path + fileName);
         if (fileOut)
         {
             fileOut << pidMax;
         }
         fileOut.close();
 
-        unique_ptr<LockFile> lockFile = unique_ptr<LockFile>(new LockFile{path, "test-aws-iot-device-client"});
+        unique_ptr<LockFile> lockFile =
+            unique_ptr<LockFile>(new LockFile{path, "test-aws-iot-device-client", thingName});
 
-        ifstream fileIn(path + filename);
+        ifstream fileIn(path + fileName);
         ASSERT_TRUE(fileIn);
 
         string storedPid;
-        if (fileIn >> storedPid)
+        string storedName;
+        if (fileIn >> storedName && fileIn >> storedPid)
         {
+            ASSERT_STREQ(thingName.c_str(), storedName.c_str());
             ASSERT_STREQ(to_string(getpid()).c_str(), storedPid.c_str());
         }
     }