Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Dump state file atomically not to corrupt it #9451

Merged
merged 3 commits into from
Aug 1, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,7 @@ else()
set(LOGROTATE_CREATE "\n\tcreate 644 ${ICINGA2_USER} ${ICINGA2_GROUP}")
endif()

find_package(Boost ${BOOST_MIN_VERSION} COMPONENTS coroutine context date_time filesystem thread system program_options regex REQUIRED)
find_package(Boost ${BOOST_MIN_VERSION} COMPONENTS coroutine context date_time filesystem iostreams thread system program_options regex REQUIRED)

# Boost.Coroutine2 (the successor of Boost.Coroutine)
# (1) doesn't even exist in old Boost versions and
Expand Down
1 change: 1 addition & 0 deletions lib/base/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ set(base_SOURCES
application.cpp application.hpp application-ti.hpp application-version.cpp application-environment.cpp
array.cpp array.hpp array-script.cpp
atomic.hpp
atomic-file.cpp atomic-file.hpp
base64.cpp base64.hpp
boolean.cpp boolean.hpp boolean-script.cpp
bulker.hpp
Expand Down
116 changes: 116 additions & 0 deletions lib/base/atomic-file.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,116 @@
/* Icinga 2 | (c) 2022 Icinga GmbH | GPLv2+ */

#include "base/atomic-file.hpp"
#include "base/exception.hpp"
#include "base/utility.hpp"
#include <utility>

#ifdef _WIN32
# include <io.h>
# include <windows.h>
#else /* _WIN32 */
# include <errno.h>
# include <unistd.h>
#endif /* _WIN32 */

using namespace icinga;

AtomicFile::AtomicFile(String path, int mode) : m_Path(std::move(path))
{
m_TempFilename = m_Path + ".tmp.XXXXXX";

#ifdef _WIN32
m_Fd = Utility::MksTemp(&m_TempFilename[0]);
#else /* _WIN32 */
m_Fd = mkstemp(&m_TempFilename[0]);
#endif /* _WIN32 */

if (m_Fd < 0) {
auto error (errno);

BOOST_THROW_EXCEPTION(posix_error()
<< boost::errinfo_api_function("mkstemp")
<< boost::errinfo_errno(error)
<< boost::errinfo_file_name(m_TempFilename));
}

try {
exceptions(failbit | badbit);

open(boost::iostreams::file_descriptor(
m_Fd,
// Rationale: https://github.com/boostorg/iostreams/issues/152
boost::iostreams::never_close_handle
));

if (chmod(m_TempFilename.CStr(), mode) < 0) {
auto error (errno);

BOOST_THROW_EXCEPTION(posix_error()
<< boost::errinfo_api_function("chmod")
<< boost::errinfo_errno(error)
<< boost::errinfo_file_name(m_TempFilename));
}
} catch (...) {
if (is_open()) {
close();
}

(void)::close(m_Fd);
(void)unlink(m_TempFilename.CStr());
throw;
}
}

AtomicFile::~AtomicFile()
{
if (is_open()) {
try {
close();
} catch (...) {
// Destructor must not throw
}
}

if (m_Fd >= 0) {
(void)::close(m_Fd);
}

if (!m_TempFilename.IsEmpty()) {
(void)unlink(m_TempFilename.CStr());
}
}

void AtomicFile::Commit()
{
flush();

auto h ((*this)->handle());

#ifdef _WIN32
if (!FlushFileBuffers(h)) {
auto err (GetLastError());

BOOST_THROW_EXCEPTION(win32_error()
<< boost::errinfo_api_function("FlushFileBuffers")
<< errinfo_win32_error(err)
<< boost::errinfo_file_name(m_TempFilename));
}
#else /* _WIN32 */
if (fsync(h)) {
auto err (errno);

BOOST_THROW_EXCEPTION(posix_error()
<< boost::errinfo_api_function("fsync")
<< boost::errinfo_errno(err)
<< boost::errinfo_file_name(m_TempFilename));
}
#endif /* _WIN32 */

close();
(void)::close(m_Fd);
m_Fd = -1;

Utility::RenameFile(m_TempFilename, m_Path);
m_TempFilename = "";
}
34 changes: 34 additions & 0 deletions lib/base/atomic-file.hpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
/* Icinga 2 | (c) 2022 Icinga GmbH | GPLv2+ */

#ifndef ATOMIC_FILE_H
#define ATOMIC_FILE_H

#include "base/string.hpp"
#include <boost/iostreams/device/file_descriptor.hpp>
#include <boost/iostreams/stream.hpp>

namespace icinga
{

/**
* Atomically replaces a file's content.
*
* @ingroup base
*/
class AtomicFile : public boost::iostreams::stream<boost::iostreams::file_descriptor>
{
public:
AtomicFile(String path, int mode);
julianbrost marked this conversation as resolved.
Show resolved Hide resolved
~AtomicFile();

void Commit();

private:
String m_Path;
String m_TempFilename;
int m_Fd;
};

}

#endif /* ATOMIC_FILE_H */
14 changes: 3 additions & 11 deletions lib/base/configobject.cpp
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
/* Icinga 2 | (c) 2012 Icinga GmbH | GPLv2+ */

#include "base/atomic-file.hpp"
#include "base/configobject.hpp"
#include "base/configobject-ti.cpp"
#include "base/configtype.hpp"
Expand Down Expand Up @@ -468,13 +469,7 @@ void ConfigObject::DumpObjects(const String& filename, int attributeTypes)
Log(LogWarning, "ConfigObject") << DiagnosticInformation(ex);
}

std::fstream fp;
String tempFilename = Utility::CreateTempFile(filename + ".tmp.XXXXXX", 0600, fp);
fp.exceptions(std::ofstream::failbit | std::ofstream::badbit);

if (!fp)
BOOST_THROW_EXCEPTION(std::runtime_error("Could not open '" + tempFilename + "' file"));

AtomicFile fp (filename, 0600);
StdioStream::Ptr sfp = new StdioStream(&fp, false);

for (const Type::Ptr& type : Type::GetAllTypes()) {
Expand Down Expand Up @@ -502,10 +497,7 @@ void ConfigObject::DumpObjects(const String& filename, int attributeTypes)
}

sfp->Close();

fp.close();

Utility::RenameFile(tempFilename, filename);
fp.Commit();
}

void ConfigObject::RestoreObject(const String& message, int attributeTypes)
Expand Down
40 changes: 40 additions & 0 deletions lib/base/exception.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -180,6 +180,13 @@ String icinga::DiagnosticInformation(const std::exception& ex, bool verbose, boo

String message = ex.what();

#ifdef _WIN32
const auto *win32_err = dynamic_cast<const win32_error *>(&ex);
if (win32_err) {
message = to_string(*win32_err);
}
#endif /* _WIN32 */

const auto *vex = dynamic_cast<const ValidationError *>(&ex);

if (message.IsEmpty())
Expand Down Expand Up @@ -424,6 +431,39 @@ std::string icinga::to_string(const StackTraceErrorInfo&)
}

#ifdef _WIN32
const char *win32_error::what() const noexcept
{
return "win32_error";
}

std::string icinga::to_string(const win32_error &e) {
julianbrost marked this conversation as resolved.
Show resolved Hide resolved
std::ostringstream msgbuf;

const char * const *func = boost::get_error_info<boost::errinfo_api_function>(e);

if (func) {
msgbuf << "Function call '" << *func << "'";
} else {
msgbuf << "Function call";
}

const std::string *fname = boost::get_error_info<boost::errinfo_file_name>(e);

if (fname) {
msgbuf << " for file '" << *fname << "'";
}

msgbuf << " failed";

const int *errnum = boost::get_error_info<errinfo_win32_error>(e);

if (errnum) {
msgbuf << " with error code " << Utility::FormatErrorNumber(*errnum);
}

return msgbuf.str();
}

std::string icinga::to_string(const errinfo_win32_error& e)
{
return "[errinfo_win32_error] = " + Utility::FormatErrorNumber(e.value()) + "\n";
Expand Down
7 changes: 6 additions & 1 deletion lib/base/exception.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,12 @@ class posix_error : virtual public std::exception, virtual public boost::excepti
};

#ifdef _WIN32
class win32_error : virtual public std::exception, virtual public boost::exception { };
class win32_error : virtual public std::exception, virtual public boost::exception {
public:
const char *what() const noexcept override;
};

std::string to_string(const win32_error& e);

struct errinfo_win32_error_;
typedef boost::error_info<struct errinfo_win32_error_, int> errinfo_win32_error;
Expand Down
8 changes: 4 additions & 4 deletions lib/base/utility.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,10 @@ class Utility

static String CreateTempFile(const String& path, int mode, std::fstream& fp);

#ifdef _WIN32
static int MksTemp(char *tmpl);
#endif /* _WIN32 */

#ifdef _WIN32
static String GetIcingaInstallPath();
static String GetIcingaDataPath();
Expand Down Expand Up @@ -185,10 +189,6 @@ class Utility
private:
Utility();

#ifdef _WIN32
static int MksTemp (char *tmpl);
#endif /* _WIN32 */

#ifdef I2_DEBUG
static double m_DebugTime;
#endif /* I2_DEBUG */
Expand Down