Commit a7a79b3c768a8e7f8aa333aede8b2baeb62a8b1a
1 parent
bc502926
Improve error handling on persistence files
Also properly fsynced it, and the dir it's in.
Showing
2 changed files
with
41 additions
and
3 deletions
persistencefile.cpp
| @@ -25,6 +25,7 @@ License along with FlashMQ. If not, see <https://www.gnu.org/licenses/>. | @@ -25,6 +25,7 @@ License along with FlashMQ. If not, see <https://www.gnu.org/licenses/>. | ||
| 25 | #include <stdexcept> | 25 | #include <stdexcept> |
| 26 | #include <stdio.h> | 26 | #include <stdio.h> |
| 27 | #include <cstring> | 27 | #include <cstring> |
| 28 | +#include <libgen.h> | ||
| 28 | 29 | ||
| 29 | #include "utils.h" | 30 | #include "utils.h" |
| 30 | #include "logger.h" | 31 | #include "logger.h" |
| @@ -39,11 +40,28 @@ PersistenceFile::PersistenceFile(const std::string &filePath) : | @@ -39,11 +40,28 @@ PersistenceFile::PersistenceFile(const std::string &filePath) : | ||
| 39 | this->filePath = filePath; | 40 | this->filePath = filePath; |
| 40 | this->filePathTemp = formatString("%s.newfile.%s", filePath.c_str(), getSecureRandomString(8).c_str()); | 41 | this->filePathTemp = formatString("%s.newfile.%s", filePath.c_str(), getSecureRandomString(8).c_str()); |
| 41 | this->filePathCorrupt = formatString("%s.corrupt.%s", filePath.c_str(), getSecureRandomString(8).c_str()); | 42 | this->filePathCorrupt = formatString("%s.corrupt.%s", filePath.c_str(), getSecureRandomString(8).c_str()); |
| 43 | + | ||
| 44 | + std::vector<char> d1(filePath.length() + 1, 0); | ||
| 45 | + std::copy(filePath.begin(), filePath.end(), d1.begin()); | ||
| 46 | + this->dirPath = std::string(dirname(d1.data())); | ||
| 42 | } | 47 | } |
| 43 | 48 | ||
| 44 | PersistenceFile::~PersistenceFile() | 49 | PersistenceFile::~PersistenceFile() |
| 45 | { | 50 | { |
| 46 | - closeFile(); | 51 | + try |
| 52 | + { | ||
| 53 | + closeFile(); | ||
| 54 | + } | ||
| 55 | + catch(std::exception &ex) | ||
| 56 | + { | ||
| 57 | + Logger::getInstance()->logf(LOG_WARNING, ex.what()); | ||
| 58 | + } | ||
| 59 | + | ||
| 60 | + if (f != nullptr) | ||
| 61 | + { | ||
| 62 | + fclose(f); // fclose was already attempted and error handled if it was possible. In case an early fault happend, we need to still make sure. | ||
| 63 | + f = nullptr; | ||
| 64 | + } | ||
| 47 | 65 | ||
| 48 | if (digestContext) | 66 | if (digestContext) |
| 49 | { | 67 | { |
| @@ -105,7 +123,6 @@ void PersistenceFile::hashFile() | @@ -105,7 +123,6 @@ void PersistenceFile::hashFile() | ||
| 105 | fseek(f, MAGIC_STRING_LENGH, SEEK_SET); | 123 | fseek(f, MAGIC_STRING_LENGH, SEEK_SET); |
| 106 | 124 | ||
| 107 | writeCheck(md_value, output_len, 1, f); | 125 | writeCheck(md_value, output_len, 1, f); |
| 108 | - fflush(f); | ||
| 109 | } | 126 | } |
| 110 | 127 | ||
| 111 | void PersistenceFile::verifyHash() | 128 | void PersistenceFile::verifyHash() |
| @@ -325,18 +342,38 @@ void PersistenceFile::closeFile() | @@ -325,18 +342,38 @@ void PersistenceFile::closeFile() | ||
| 325 | return; | 342 | return; |
| 326 | 343 | ||
| 327 | if (openMode == FileMode::write) | 344 | if (openMode == FileMode::write) |
| 345 | + { | ||
| 328 | hashFile(); | 346 | hashFile(); |
| 329 | 347 | ||
| 348 | + if (fflush(f) != 0) | ||
| 349 | + { | ||
| 350 | + std::string msg(strerror(errno)); | ||
| 351 | + throw std::runtime_error(formatString("Flush of '%s' failed: %s.", this->filePathTemp.c_str(), msg.c_str())); | ||
| 352 | + } | ||
| 353 | + | ||
| 354 | + fsync(f->_fileno); | ||
| 355 | + } | ||
| 356 | + | ||
| 330 | if (f != nullptr) | 357 | if (f != nullptr) |
| 331 | { | 358 | { |
| 332 | - fclose(f); | 359 | + FILE *f2 = f; |
| 333 | f = nullptr; | 360 | f = nullptr; |
| 361 | + | ||
| 362 | + if (fclose(f2) < 0) | ||
| 363 | + { | ||
| 364 | + std::string msg(strerror(errno)); | ||
| 365 | + throw std::runtime_error(formatString("Close of '%s' failed: %s.", this->filePathTemp.c_str(), msg.c_str())); | ||
| 366 | + } | ||
| 334 | } | 367 | } |
| 335 | 368 | ||
| 336 | if (openMode == FileMode::write && !filePathTemp.empty() && ! filePath.empty()) | 369 | if (openMode == FileMode::write && !filePathTemp.empty() && ! filePath.empty()) |
| 337 | { | 370 | { |
| 338 | if (rename(filePathTemp.c_str(), filePath.c_str()) < 0) | 371 | if (rename(filePathTemp.c_str(), filePath.c_str()) < 0) |
| 339 | throw std::runtime_error(formatString("Saving '%s' failed: rename of temp file to target failed with: %s", filePath.c_str(), strerror(errno))); | 372 | throw std::runtime_error(formatString("Saving '%s' failed: rename of temp file to target failed with: %s", filePath.c_str(), strerror(errno))); |
| 373 | + | ||
| 374 | + int dir_fd = open(this->dirPath.c_str(), O_RDONLY); | ||
| 375 | + fsync(dir_fd); | ||
| 376 | + close(dir_fd); | ||
| 340 | } | 377 | } |
| 341 | } | 378 | } |
| 342 | 379 |
persistencefile.h
| @@ -46,6 +46,7 @@ class PersistenceFile | @@ -46,6 +46,7 @@ class PersistenceFile | ||
| 46 | std::string filePath; | 46 | std::string filePath; |
| 47 | std::string filePathTemp; | 47 | std::string filePathTemp; |
| 48 | std::string filePathCorrupt; | 48 | std::string filePathCorrupt; |
| 49 | + std::string dirPath; | ||
| 49 | 50 | ||
| 50 | EVP_MD_CTX *digestContext = nullptr; | 51 | EVP_MD_CTX *digestContext = nullptr; |
| 51 | const EVP_MD *sha512 = EVP_sha512(); | 52 | const EVP_MD *sha512 = EVP_sha512(); |