diff --git a/doc/xml/release.xml b/doc/xml/release.xml index 1223495ac..298fde25b 100644 --- a/doc/xml/release.xml +++ b/doc/xml/release.xml @@ -158,14 +158,19 @@ - + + + + + + -

Add lockRead*() functions for reading locks from another process.

+

Lock module refactoring.

 diff --git a/src/command/control/stop.c b/src/command/control/stop.c index 270aeba17..a0d254cdc 100644 --- a/src/command/control/stop.c +++ b/src/command/control/stop.c @@ -69,21 +69,18 @@ cmdStop(void) lockFile = strNewFmt("%s/%s", strZ(lockPath), strZ(lockFile)); LockReadResult lockRead = lockReadFileP(lockFile, .remove = true); - // If we cannot open the lock file for any reason then warn and continue to next file - if (lockRead.status != lockReadStatusValid && lockRead.status != lockReadStatusUnlocked) + // If we cannot read the lock file for any reason then warn and continue to next file + if (lockRead.status != lockReadStatusValid) { - LOG_WARN_FMT("unable to open lock file %s", strZ(lockFile)); + LOG_WARN_FMT("unable to read lock file %s", strZ(lockFile)); continue; } - // The file is locked so that means there is a running process -- send a term signal to the process if valid - if (lockRead.data.processId != 0) - { - if (kill(lockRead.data.processId, SIGTERM) != 0) - LOG_WARN_FMT("unable to send term signal to process %d", lockRead.data.processId); - else - LOG_INFO_FMT("sent term signal to process %d", lockRead.data.processId); - } + // The lock file is valid so that means there is a running process -- send a term signal to the process + if (kill(lockRead.data.processId, SIGTERM) != 0) + LOG_WARN_FMT("unable to send term signal to process %d", lockRead.data.processId); + else + LOG_INFO_FMT("sent term signal to process %d", lockRead.data.processId); } } } diff --git a/src/common/lock.c b/src/common/lock.c index 2cc7ccbd5..17f55262b 100644 --- a/src/common/lock.c +++ b/src/common/lock.c @@ -92,29 +92,36 @@ lockReadFileData(const String *const lockFile, const int fd) LockData result = {0}; - MEM_CONTEXT_TEMP_BEGIN() + TRY_BEGIN() { - // Read contents of file - Buffer *const buffer = bufNew(LOCK_BUFFER_SIZE); - IoWrite *const write = ioBufferWriteNewOpen(buffer); - - ioCopyP(ioFdReadNewOpen(lockFile, fd, 0), write); - ioWriteClose(write); - - // Parse the file - const StringList *const parse = strLstNewSplitZ(strNewBuf(buffer), LF_Z); - - MEM_CONTEXT_PRIOR_BEGIN() + MEM_CONTEXT_TEMP_BEGIN() { - if (!strEmpty(strTrim(strLstGet(parse, 0)))) - result.processId = cvtZToInt(strZ(strLstGet(parse, 0))); + // Read contents of file + Buffer *const buffer = bufNew(LOCK_BUFFER_SIZE); + IoWrite *const write = ioBufferWriteNewOpen(buffer); - if (strLstSize(parse) == 3) - result.execId = strDup(strLstGet(parse, 1)); + ioCopyP(ioFdReadNewOpen(lockFile, fd, 0), write); + ioWriteClose(write); + + // Parse the file + const StringList *const parse = strLstNewSplitZ(strNewBuf(buffer), LF_Z); + + MEM_CONTEXT_PRIOR_BEGIN() + { + if (strLstSize(parse) == 3) + result.execId = strDup(strLstGet(parse, 1)); + + result.processId = cvtZToInt(strZ(strLstGet(parse, 0))); + } + MEM_CONTEXT_PRIOR_END(); } - MEM_CONTEXT_PRIOR_END(); + MEM_CONTEXT_TEMP_END(); } - MEM_CONTEXT_TEMP_END(); + CATCH_ANY() + { + THROWP_FMT(errorType(), "unable to read lock file '%s': %s", strZ(lockFile), errorMessage()); + } + TRY_END(); FUNCTION_LOG_RETURN_STRUCT(result); } @@ -286,6 +293,13 @@ lockAcquireFile(const String *const lockFile, const TimeMSec lockTimeout, const { execId = lockReadFileData(lockFile, result).execId; } + CATCH_ANY() + { + // Any errors will be reported as unable to acquire a lock. If a process is trying to get a lock but is not + // synchronized with the process holding the actual lock, it is possible that it could see a short read or + // have some other problem reading. Reporting the error will likely be misleading when the actual problem is + // that another process owns the lock file. + } FINALLY() { close(result); diff --git a/test/src/module/command/controlTest.c b/test/src/module/command/controlTest.c index 6da70a5ee..96ac70f80 100644 --- a/test/src/module/command/controlTest.c +++ b/test/src/module/command/controlTest.c @@ -136,14 +136,14 @@ testRun(void) TEST_STORAGE_EXISTS(hrnStorage, "lock/db" STOP_FILE_EXT, .remove = true, .comment = "stanza stop file created, remove"); // ------------------------------------------------------------------------------------------------------------------------- - TEST_TITLE("unable to open lock file"); + TEST_TITLE("unable to read lock file"); HRN_STORAGE_PUT_EMPTY( hrnStorage, "lock/db-archive" LOCK_FILE_EXT, .modeFile = 0222, .comment = "create a lock file that cannot be opened"); - TEST_RESULT_VOID(cmdStop(), "stanza, create stop file but unable to open lock file"); + TEST_RESULT_VOID(cmdStop(), "stanza, create stop file but unable to read lock file"); TEST_STORAGE_EXISTS(hrnStorage, "lock/db" STOP_FILE_EXT, .comment = "stanza stop file created"); - TEST_RESULT_LOG("P00 WARN: unable to open lock file " HRN_PATH "/lock/db-archive" LOCK_FILE_EXT); + TEST_RESULT_LOG("P00 WARN: unable to read lock file " HRN_PATH "/lock/db-archive" LOCK_FILE_EXT); HRN_STORAGE_PATH_REMOVE(hrnStorage, "lock", .recurse = true, .errorOnMissing = true, .comment = "remove the lock path"); // ------------------------------------------------------------------------------------------------------------------------- @@ -155,6 +155,8 @@ testRun(void) hrnStorage, "lock", "db" STOP_FILE_EXT "\n", .comment = "stanza stop file created, no other process lock, lock file was removed"); + TEST_RESULT_LOG_FMT("P00 WARN: unable to read lock file " HRN_PATH "/lock/db-backup.lock"); + // ------------------------------------------------------------------------------------------------------------------------- TEST_TITLE("empty lock file with another process lock, processId == NULL"); @@ -192,6 +194,8 @@ testRun(void) // Notify child to release lock HRN_FORK_PARENT_NOTIFY_PUT(0); + + TEST_RESULT_LOG_FMT("P00 WARN: unable to read lock file " HRN_PATH "/lock/db-backup.lock"); } HRN_FORK_PARENT_END(); } @@ -234,6 +238,8 @@ testRun(void) // Notify child to release lock HRN_FORK_PARENT_NOTIFY_PUT(0); + + TEST_RESULT_LOG_FMT("P00 WARN: unable to read lock file " HRN_PATH "/lock/db-backup.lock"); } HRN_FORK_PARENT_END(); } @@ -336,9 +342,13 @@ testRun(void) TEST_STORAGE_EXISTS(hrnStorage, "lock/all" STOP_FILE_EXT, .comment = "stanza stop file created"); TEST_STORAGE_LIST( hrnStorage, "lock", "all" STOP_FILE_EXT "\n" "db-junk.txt\n", .comment = "stop file created, all lock files processed"); - TEST_RESULT_LOG(""); HRN_STORAGE_PATH_REMOVE(hrnStorage, "lock", .recurse = true, .errorOnMissing = true, .comment = "remove the lock path"); + TEST_RESULT_LOG_FMT( + "P00 WARN: unable to read lock file " HRN_PATH "/lock/db-archive.lock\n" + "P00 WARN: unable to read lock file " HRN_PATH "/lock/db-backup.lock\n" + "P00 WARN: unable to read lock file " HRN_PATH "/lock/db1-backup.lock"); + // ------------------------------------------------------------------------------------------------------------------------- TEST_TITLE("stanza, force stop = process only stanza lock files, ignore other stanza lock files and other files"); @@ -355,8 +365,11 @@ testRun(void) TEST_STORAGE_LIST( hrnStorage, "lock", "db-junk.txt\ndb" STOP_FILE_EXT "\n" "db1-backup" LOCK_FILE_EXT "\n", .comment = "stop file created, stanza lock file was removed, other stanza lock and other files remain"); - TEST_RESULT_LOG(""); HRN_STORAGE_PATH_REMOVE(hrnStorage, "lock", .recurse = true, .errorOnMissing = true, .comment = "remove the lock path"); + + TEST_RESULT_LOG_FMT( + "P00 WARN: unable to read lock file " HRN_PATH "/lock/db-archive.lock\n" + "P00 WARN: unable to read lock file " HRN_PATH "/lock/db-backup.lock"); } FUNCTION_HARNESS_RETURN_VOID();