Skip to content

Commit

Permalink
Update lockfile handler (#332)
Browse files Browse the repository at this point in the history 
* fixes bug of device client not starting when filesystem requirements are not met
  • Loading branch information
@RogerZhongAWS
RogerZhongAWS authored Dec 14, 2022
1 parent fb17eae commit bb7ff67
Show file tree
Hide file tree
Showing 6 changed files with 85 additions and 42 deletions.
33 changes: 30 additions & 3 deletions integration-tests/entry-point.sh
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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}

Expand Down Expand Up @@ -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 &

Expand Down
9 changes: 0 additions & 9 deletions source/config/Config.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand Down
13 changes: 11 additions & 2 deletions source/main.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -316,7 +322,7 @@ int main(int argc, char *argv[])
*/
if (!init(argc, argv))
{
return -1;
return 1;
}
features = make_shared<FeatureRegistry>();

Expand Down Expand Up @@ -537,6 +543,9 @@ int main(int argc, char *argv[])
case SIGINT:
shutdown();
break;
case SIGTERM:
shutdown();
break;
case SIGHUP:
resourceManager->dumpMemTrace();
break;
Expand Down
26 changes: 15 additions & 11 deletions source/util/LockFile.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -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()
Expand Down
2 changes: 1 addition & 1 deletion source/util/LockFile.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
44 changes: 28 additions & 16 deletions test/util/TestLockFile.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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());
}
}
Expand Down

0 comments on commit bb7ff67

Please sign in to comment.