mirror of
https://github.com/Icinga/icinga2.git
synced 2026-05-28 04:12:13 -04:00
Fix PerfdataWriterConnection segfaults on non-X86 architectures
The issue is that std::promise internally also used thread local storage, in a call to `std::call_once` in `std::promise::set_value()`. The theory is that since all paths in `Send()` run this `std::call_once` routine and from then on, then Coroutine function looks like a normal function, the compiler inlined `set_value()` and moved the common parts of it to a common location for all paths before the suspension point in WriteMessage(yc). When finally the coroutine is resumes, it is likely that that happens under a different thread, which still has `__once_callable` in `std::call_once` set as `nullptr`, leading to the segmentation fault. The fix is to not use std::promise across coroutine suspension points and instead reimplement the functionality we required from it in a small helper class `SyncResult` that does not require any thread local storage.
This commit is contained in:
parent
3cc5ee2122
commit
792c70393a
2 changed files with 61 additions and 10 deletions
|
|
@ -66,7 +66,7 @@ void PerfdataWriterConnection::Disconnect()
|
|||
return;
|
||||
}
|
||||
|
||||
std::promise<void> promise;
|
||||
SyncResult<void> ret;
|
||||
|
||||
IoEngine::SpawnCoroutine(m_Strand, [&](boost::asio::yield_context yc) {
|
||||
try {
|
||||
|
|
@ -90,13 +90,13 @@ void PerfdataWriterConnection::Disconnect()
|
|||
m_ReconnectTimer.cancel();
|
||||
|
||||
Disconnect(std::move(yc));
|
||||
promise.set_value();
|
||||
ret.SetValue();
|
||||
} catch (const std::exception& ex) {
|
||||
promise.set_exception(std::current_exception());
|
||||
ret.SetException(std::current_exception());
|
||||
}
|
||||
});
|
||||
|
||||
promise.get_future().get();
|
||||
ret.Get();
|
||||
}
|
||||
|
||||
AsioTlsOrTcpStream PerfdataWriterConnection::MakeStream() const
|
||||
|
|
|
|||
|
|
@ -10,6 +10,7 @@
|
|||
#include <boost/beast/http/message.hpp>
|
||||
#include <boost/beast/http/string_body.hpp>
|
||||
#include <future>
|
||||
#include <utility>
|
||||
|
||||
namespace icinga {
|
||||
|
||||
|
|
@ -21,6 +22,56 @@ class PerfdataWriterConnection final : public Object
|
|||
static constexpr auto InitialRetryWait = 50ms;
|
||||
static constexpr auto FinalRetryWait = 32s;
|
||||
|
||||
template<typename T>
|
||||
class SyncResult
|
||||
{
|
||||
using ValueType = std::variant<std::monostate, std::conditional_t<std::is_void_v<T>, bool, T>, std::exception_ptr>;
|
||||
|
||||
public:
|
||||
template<typename U, typename V = T, typename = std::enable_if_t<!std::is_void_v<V>>>
|
||||
void SetValue(U&& v)
|
||||
{
|
||||
std::lock_guard lock(m_Mutex);
|
||||
m_Value = std::forward<U>(v);
|
||||
m_Cv.notify_one();
|
||||
}
|
||||
|
||||
template<typename V = T, typename = std::enable_if_t<std::is_void_v<V>>>
|
||||
void SetValue()
|
||||
{
|
||||
std::lock_guard lock(m_Mutex);
|
||||
m_Value = true;
|
||||
m_Cv.notify_one();
|
||||
}
|
||||
|
||||
void SetException(std::exception_ptr ep)
|
||||
{
|
||||
std::lock_guard lock(m_Mutex);
|
||||
m_Value = ValueType{ep};
|
||||
m_Cv.notify_one();
|
||||
}
|
||||
|
||||
T Get()
|
||||
{
|
||||
std::unique_lock l(m_Mutex);
|
||||
m_Cv.wait(l, [&] { return !std::holds_alternative<std::monostate>(m_Value); });
|
||||
if (std::holds_alternative<std::exception_ptr>(m_Value)) {
|
||||
std::rethrow_exception(std::get<std::exception_ptr>(m_Value));
|
||||
}
|
||||
|
||||
if constexpr (std::is_void_v<T>) {
|
||||
return;
|
||||
} else {
|
||||
return std::move(std::get<T>(m_Value));
|
||||
}
|
||||
}
|
||||
|
||||
private:
|
||||
std::mutex m_Mutex;
|
||||
std::condition_variable m_Cv;
|
||||
ValueType m_Value;
|
||||
};
|
||||
|
||||
public:
|
||||
DECLARE_PTR_TYPEDEFS(PerfdataWriterConnection);
|
||||
|
||||
|
|
@ -66,7 +117,7 @@ public:
|
|||
}
|
||||
|
||||
using RetType = decltype(WriteMessage(std::declval<Buffer>(), std::declval<boost::asio::yield_context>()));
|
||||
std::promise<RetType> promise;
|
||||
SyncResult<RetType> ret;
|
||||
|
||||
IoEngine::SpawnCoroutine(m_Strand, [&](boost::asio::yield_context yc) {
|
||||
while (true) {
|
||||
|
|
@ -75,16 +126,16 @@ public:
|
|||
|
||||
if constexpr (std::is_void_v<RetType>) {
|
||||
WriteMessage(std::forward<Buffer>(buf), yc);
|
||||
promise.set_value();
|
||||
ret.SetValue();
|
||||
} else {
|
||||
promise.set_value(WriteMessage(std::forward<Buffer>(buf), yc));
|
||||
ret.SetValue(WriteMessage(std::forward<Buffer>(buf), yc));
|
||||
}
|
||||
|
||||
m_RetryTimeout = InitialRetryWait;
|
||||
return;
|
||||
} catch (const std::exception& ex) {
|
||||
if (m_Stopped) {
|
||||
promise.set_exception(std::make_exception_ptr(Stopped{}));
|
||||
ret.SetException(std::make_exception_ptr(Stopped{}));
|
||||
return;
|
||||
}
|
||||
|
||||
|
|
@ -98,14 +149,14 @@ public:
|
|||
try {
|
||||
BackoffWait(yc);
|
||||
} catch (const std::exception&) {
|
||||
promise.set_exception(std::make_exception_ptr(Stopped{}));
|
||||
ret.SetException(std::make_exception_ptr(Stopped{}));
|
||||
return;
|
||||
}
|
||||
}
|
||||
}
|
||||
});
|
||||
|
||||
return promise.get_future().get();
|
||||
return ret.Get();
|
||||
}
|
||||
|
||||
void Disconnect();
|
||||
|
|
|
|||
Loading…
Reference in a new issue