From 11ab8690164b76504facf6438e735c941283fbc3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20Aleksandrovi=C4=8D=20Klimov?= Date: Thu, 2 Jan 2025 15:43:13 +0100 Subject: [PATCH 1/8] Bump Boost shipped for Windows to v1.87 --- doc/win-dev.ps1 | 2 +- tools/win32/configure-dev.ps1 | 4 ++-- tools/win32/configure.ps1 | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/doc/win-dev.ps1 b/doc/win-dev.ps1 index f64017bb0..e3b4e5955 100644 --- a/doc/win-dev.ps1 +++ b/doc/win-dev.ps1 @@ -13,7 +13,7 @@ function ThrowOnNativeFailure { $VsVersion = 2019 $MsvcVersion = '14.2' -$BoostVersion = @(1, 86, 0) +$BoostVersion = @(1, 87, 0) $OpensslVersion = '3_0_15' switch ($Env:BITS) { diff --git a/tools/win32/configure-dev.ps1 b/tools/win32/configure-dev.ps1 index a2caeb201..7729dc58f 100644 --- a/tools/win32/configure-dev.ps1 +++ b/tools/win32/configure-dev.ps1 @@ -34,10 +34,10 @@ if (-not (Test-Path env:OPENSSL_ROOT_DIR)) { $env:OPENSSL_ROOT_DIR = 'c:\local\OpenSSL-Win64' } if (-not (Test-Path env:BOOST_ROOT)) { - $env:BOOST_ROOT = 'c:\local\boost_1_86_0' + $env:BOOST_ROOT = 'c:\local\boost_1_87_0' } if (-not (Test-Path env:BOOST_LIBRARYDIR)) { - $env:BOOST_LIBRARYDIR = 'c:\local\boost_1_86_0\lib64-msvc-14.2' + $env:BOOST_LIBRARYDIR = 'c:\local\boost_1_87_0\lib64-msvc-14.2' } if (-not (Test-Path env:FLEX_BINARY)) { $env:FLEX_BINARY = 'C:\ProgramData\chocolatey\bin\win_flex.exe' diff --git a/tools/win32/configure.ps1 b/tools/win32/configure.ps1 index 7ab3dbc52..52d8628a1 100644 --- a/tools/win32/configure.ps1 +++ b/tools/win32/configure.ps1 @@ -36,10 +36,10 @@ if (-not (Test-Path env:OPENSSL_ROOT_DIR)) { $env:OPENSSL_ROOT_DIR = "c:\local\OpenSSL_3_0_15-Win${env:BITS}" } if (-not (Test-Path env:BOOST_ROOT)) { - $env:BOOST_ROOT = "c:\local\boost_1_86_0-Win${env:BITS}" + $env:BOOST_ROOT = "c:\local\boost_1_87_0-Win${env:BITS}" } if (-not (Test-Path env:BOOST_LIBRARYDIR)) { - $env:BOOST_LIBRARYDIR = "c:\local\boost_1_86_0-Win${env:BITS}\lib${env:BITS}-msvc-14.2" + $env:BOOST_LIBRARYDIR = "c:\local\boost_1_87_0-Win${env:BITS}\lib${env:BITS}-msvc-14.2" } if (-not (Test-Path env:FLEX_BINARY)) { $env:FLEX_BINARY = 'C:\ProgramData\chocolatey\bin\win_flex.exe' From 7bd35d8c6b7658170835de0fa623e3a4099569fb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20Aleksandrovi=C4=8D=20Klimov?= Date: Tue, 7 Jan 2025 15:53:20 +0100 Subject: [PATCH 2/8] Don't use boost::asio::ip::tcp::resolver::query It was removed in Boost 1.87. --- lib/base/tcpsocket.hpp | 6 ++---- lib/remote/apilistener.cpp | 4 +--- 2 files changed, 3 insertions(+), 7 deletions(-) diff --git a/lib/base/tcpsocket.hpp b/lib/base/tcpsocket.hpp index 471ad8d23..1cf1a2350 100644 --- a/lib/base/tcpsocket.hpp +++ b/lib/base/tcpsocket.hpp @@ -41,8 +41,7 @@ void Connect(Socket& socket, const String& node, const String& service) using boost::asio::ip::tcp; tcp::resolver resolver (IoEngine::Get().GetIoContext()); - tcp::resolver::query query (node, service); - auto result (resolver.resolve(query)); + auto result (resolver.resolve(node.CStr(), service.CStr())); auto current (result.begin()); for (;;) { @@ -72,8 +71,7 @@ void Connect(Socket& socket, const String& node, const String& service, boost::a using boost::asio::ip::tcp; tcp::resolver resolver (IoEngine::Get().GetIoContext()); - tcp::resolver::query query (node, service); - auto result (resolver.async_resolve(query, yc)); + auto result (resolver.async_resolve(node.CStr(), service.CStr(), yc)); auto current (result.begin()); for (;;) { diff --git a/lib/remote/apilistener.cpp b/lib/remote/apilistener.cpp index 772cd2df2..6c684e9f1 100644 --- a/lib/remote/apilistener.cpp +++ b/lib/remote/apilistener.cpp @@ -439,9 +439,7 @@ bool ApiListener::AddListener(const String& node, const String& service) try { tcp::resolver resolver (io); - tcp::resolver::query query (node, service, tcp::resolver::query::passive); - - auto result (resolver.resolve(query)); + auto result (resolver.resolve(node.CStr(), service.CStr(), tcp::resolver::passive)); auto current (result.begin()); for (;;) { From 011c67964ee2ab4c9a21f8aab322663e3bf5c317 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20Aleksandrovi=C4=8D=20Klimov?= Date: Tue, 7 Jan 2025 17:53:42 +0100 Subject: [PATCH 3/8] Don't use boost::asio::io_context::strand method removed in Boost 1.87 --- lib/remote/jsonrpcconnection.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/remote/jsonrpcconnection.cpp b/lib/remote/jsonrpcconnection.cpp index b8ae22aaa..889d4452c 100644 --- a/lib/remote/jsonrpcconnection.cpp +++ b/lib/remote/jsonrpcconnection.cpp @@ -212,7 +212,7 @@ void JsonRpcConnection::SendMessage(const Dictionary::Ptr& message) Ptr keepAlive (this); - m_IoStrand.post([this, keepAlive, message]() { SendMessageInternal(message); }); + boost::asio::post(m_IoStrand, [this, keepAlive, message] { SendMessageInternal(message); }); } void JsonRpcConnection::SendRawMessage(const String& message) @@ -223,7 +223,7 @@ void JsonRpcConnection::SendRawMessage(const String& message) Ptr keepAlive (this); - m_IoStrand.post([this, keepAlive, message]() { + boost::asio::post(m_IoStrand, [this, keepAlive, message] { if (m_ShuttingDown) { return; } From 0662f2b7193693ba1129690ac004af7f866470bb Mon Sep 17 00:00:00 2001 From: "Alexander A. Klimov" Date: Fri, 7 Mar 2025 16:22:50 +0100 Subject: [PATCH 4/8] In a coroutine, re-throw everything ex. std::exception (and inheritors) not just boost::coroutines::detail::forced_unwind. This is needed because as of Boost 1.87, boost::asio::spawn() uses Fiber, not Coroutine v1. https://github.com/boostorg/asio/commit/df973a85ed69f021 This is safe because every actual exception shall inherit from std::exception. Except forced_unwind and its Fiber equivalent, so that `catch(const std::exception&)` doesn't catch them and only them. --- lib/base/io-engine.hpp | 13 ++++++---- lib/icingadb/redisconnection.cpp | 41 ++++---------------------------- lib/icingadb/redisconnection.hpp | 8 ++----- 3 files changed, 14 insertions(+), 48 deletions(-) diff --git a/lib/base/io-engine.hpp b/lib/base/io-engine.hpp index 0350d45b8..55a06fb6a 100644 --- a/lib/base/io-engine.hpp +++ b/lib/base/io-engine.hpp @@ -106,14 +106,17 @@ public: try { f(yc); - } catch (const boost::coroutines::detail::forced_unwind &) { - // Required for proper stack unwinding when coroutines are destroyed. - // https://github.com/boostorg/coroutine/issues/39 - throw; } catch (const std::exception& ex) { Log(LogCritical, "IoEngine") << "Exception in coroutine: " << DiagnosticInformation(ex); } catch (...) { - Log(LogCritical, "IoEngine", "Exception in coroutine!"); + try { + Log(LogCritical, "IoEngine", "Exception in coroutine!"); + } catch (...) { + } + + // Required for proper stack unwinding when coroutines are destroyed. + // https://github.com/boostorg/coroutine/issues/39 + throw; } }, boost::coroutines::attributes(GetCoroutineStackSize()) // Set a pre-defined stack size. diff --git a/lib/icingadb/redisconnection.cpp b/lib/icingadb/redisconnection.cpp index a6b82187d..c1f73f5a0 100644 --- a/lib/icingadb/redisconnection.cpp +++ b/lib/icingadb/redisconnection.cpp @@ -377,8 +377,6 @@ void RedisConnection::Connect(asio::yield_context& yc) } break; - } catch (const boost::coroutines::detail::forced_unwind&) { - throw; } catch (const std::exception& ex) { Log(LogCritical, "IcingaDB") << "Cannot connect to " << m_Host << ":" << m_Port << ": " << ex.what(); @@ -408,17 +406,10 @@ void RedisConnection::ReadLoop(asio::yield_context& yc) for (auto i (item.Amount); i; --i) { ReadOne(yc); } - } catch (const boost::coroutines::detail::forced_unwind&) { - throw; } catch (const std::exception& ex) { Log(LogCritical, "IcingaDB") << "Error during receiving the response to a query which has been fired and forgotten: " << ex.what(); - continue; - } catch (...) { - Log(LogCritical, "IcingaDB") - << "Error during receiving the response to a query which has been fired and forgotten"; - continue; } @@ -432,9 +423,7 @@ void RedisConnection::ReadLoop(asio::yield_context& yc) try { reply = ReadOne(yc); - } catch (const boost::coroutines::detail::forced_unwind&) { - throw; - } catch (...) { + } catch (const std::exception&) { promise.set_exception(std::current_exception()); continue; @@ -455,9 +444,7 @@ void RedisConnection::ReadLoop(asio::yield_context& yc) for (auto i (item.Amount); i; --i) { try { replies.emplace_back(ReadOne(yc)); - } catch (const boost::coroutines::detail::forced_unwind&) { - throw; - } catch (...) { + } catch (const std::exception&) { promise.set_exception(std::current_exception()); break; } @@ -551,19 +538,11 @@ void RedisConnection::WriteItem(boost::asio::yield_context& yc, RedisConnection: try { WriteOne(item, yc); - } catch (const boost::coroutines::detail::forced_unwind&) { - throw; } catch (const std::exception& ex) { Log msg (LogCritical, "IcingaDB", "Error during sending query"); LogQuery(item, msg); msg << " which has been fired and forgotten: " << ex.what(); - return; - } catch (...) { - Log msg (LogCritical, "IcingaDB", "Error during sending query"); - LogQuery(item, msg); - msg << " which has been fired and forgotten"; - return; } @@ -587,19 +566,11 @@ void RedisConnection::WriteItem(boost::asio::yield_context& yc, RedisConnection: WriteOne(query, yc); ++i; } - } catch (const boost::coroutines::detail::forced_unwind&) { - throw; } catch (const std::exception& ex) { Log msg (LogCritical, "IcingaDB", "Error during sending query"); LogQuery(item[i], msg); msg << " which has been fired and forgotten: " << ex.what(); - return; - } catch (...) { - Log msg (LogCritical, "IcingaDB", "Error during sending query"); - LogQuery(item[i], msg); - msg << " which has been fired and forgotten"; - return; } @@ -618,9 +589,7 @@ void RedisConnection::WriteItem(boost::asio::yield_context& yc, RedisConnection: try { WriteOne(item.first, yc); - } catch (const boost::coroutines::detail::forced_unwind&) { - throw; - } catch (...) { + } catch (const std::exception&) { item.second.set_exception(std::current_exception()); return; @@ -645,9 +614,7 @@ void RedisConnection::WriteItem(boost::asio::yield_context& yc, RedisConnection: for (auto& query : item.first) { WriteOne(query, yc); } - } catch (const boost::coroutines::detail::forced_unwind&) { - throw; - } catch (...) { + } catch (const std::exception&) { item.second.set_exception(std::current_exception()); return; diff --git a/lib/icingadb/redisconnection.hpp b/lib/icingadb/redisconnection.hpp index 3f963f3d3..acc6e4381 100644 --- a/lib/icingadb/redisconnection.hpp +++ b/lib/icingadb/redisconnection.hpp @@ -389,9 +389,7 @@ RedisConnection::Reply RedisConnection::ReadOne(StreamPtr& stream, boost::asio:: try { return ReadRESP(*strm, yc); - } catch (const boost::coroutines::detail::forced_unwind&) { - throw; - } catch (...) { + } catch (const std::exception&) { if (m_Connecting.exchange(false)) { m_Connected.store(false); stream = nullptr; @@ -427,9 +425,7 @@ void RedisConnection::WriteOne(StreamPtr& stream, RedisConnection::Query& query, try { WriteRESP(*strm, query, yc); strm->async_flush(yc); - } catch (const boost::coroutines::detail::forced_unwind&) { - throw; - } catch (...) { + } catch (const std::exception&) { if (m_Connecting.exchange(false)) { m_Connected.store(false); stream = nullptr; From fb2b2e2d5b32a02309aa1c50ae5e9fb873922988 Mon Sep 17 00:00:00 2001 From: "Alexander A. Klimov" Date: Fri, 7 Mar 2025 15:47:37 +0100 Subject: [PATCH 5/8] Don't use removed boost::asio::spawn() overload if Boost >= v1.87 --- lib/base/io-engine.hpp | 13 +++++++++++++ test/base-io-engine.cpp | 10 +++++----- 2 files changed, 18 insertions(+), 5 deletions(-) diff --git a/lib/base/io-engine.hpp b/lib/base/io-engine.hpp index 55a06fb6a..2083b62f8 100644 --- a/lib/base/io-engine.hpp +++ b/lib/base/io-engine.hpp @@ -16,11 +16,16 @@ #include #include #include +#include #include #include #include #include +#if BOOST_VERSION >= 108700 +# include +#endif // BOOST_VERSION >= 108700 + namespace icinga { @@ -102,6 +107,10 @@ public: static void SpawnCoroutine(Handler& h, Function f) { boost::asio::spawn(h, +#if BOOST_VERSION >= 108700 + std::allocator_arg, + boost::context::fixedsize_stack(GetCoroutineStackSize()), +#endif // BOOST_VERSION >= 108700 [f](boost::asio::yield_context yc) { try { @@ -119,7 +128,11 @@ public: throw; } }, +#if BOOST_VERSION >= 108700 + boost::asio::detached +#else // BOOST_VERSION >= 108700 boost::coroutines::attributes(GetCoroutineStackSize()) // Set a pre-defined stack size. +#endif // BOOST_VERSION >= 108700 ); } diff --git a/test/base-io-engine.cpp b/test/base-io-engine.cpp index 869688b1a..3a251b1b4 100644 --- a/test/base-io-engine.cpp +++ b/test/base-io-engine.cpp @@ -17,7 +17,7 @@ BOOST_AUTO_TEST_CASE(timeout_run) boost::asio::io_context::strand strand (io); int called = 0; - boost::asio::spawn(strand, [&](boost::asio::yield_context yc) { + IoEngine::SpawnCoroutine(strand, [&](boost::asio::yield_context yc) { boost::asio::deadline_timer timer (io); Timeout timeout (strand, boost::posix_time::millisec(300), [&called] { ++called; }); @@ -44,7 +44,7 @@ BOOST_AUTO_TEST_CASE(timeout_cancelled) boost::asio::io_context::strand strand (io); int called = 0; - boost::asio::spawn(strand, [&](boost::asio::yield_context yc) { + IoEngine::SpawnCoroutine(strand, [&](boost::asio::yield_context yc) { boost::asio::deadline_timer timer (io); Timeout timeout (strand, boost::posix_time::millisec(300), [&called] { ++called; }); @@ -71,7 +71,7 @@ BOOST_AUTO_TEST_CASE(timeout_scope) boost::asio::io_context::strand strand (io); int called = 0; - boost::asio::spawn(strand, [&](boost::asio::yield_context yc) { + IoEngine::SpawnCoroutine(strand, [&](boost::asio::yield_context yc) { boost::asio::deadline_timer timer (io); { @@ -100,7 +100,7 @@ BOOST_AUTO_TEST_CASE(timeout_due_cancelled) boost::asio::io_context::strand strand (io); int called = 0; - boost::asio::spawn(strand, [&](boost::asio::yield_context yc) { + IoEngine::SpawnCoroutine(strand, [&](boost::asio::yield_context yc) { boost::asio::deadline_timer timer (io); Timeout timeout (strand, boost::posix_time::millisec(300), [&called] { ++called; }); @@ -131,7 +131,7 @@ BOOST_AUTO_TEST_CASE(timeout_due_scope) boost::asio::io_context::strand strand (io); int called = 0; - boost::asio::spawn(strand, [&](boost::asio::yield_context yc) { + IoEngine::SpawnCoroutine(strand, [&](boost::asio::yield_context yc) { boost::asio::deadline_timer timer (io); { From ccfc72267f5c4bddad6779bb30c93a504f286500 Mon Sep 17 00:00:00 2001 From: Julian Brost Date: Fri, 11 Apr 2025 12:27:25 +0200 Subject: [PATCH 6/8] Prefer icinga::String::GetData() over icinga::String::CStr() Creating the string_view from the std::string (as returned by GetData()) uses the stored length instead of having to detect it by finding '\0'. --- lib/base/tcpsocket.hpp | 4 ++-- lib/remote/apilistener.cpp | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/base/tcpsocket.hpp b/lib/base/tcpsocket.hpp index 1cf1a2350..7e64d57a5 100644 --- a/lib/base/tcpsocket.hpp +++ b/lib/base/tcpsocket.hpp @@ -41,7 +41,7 @@ void Connect(Socket& socket, const String& node, const String& service) using boost::asio::ip::tcp; tcp::resolver resolver (IoEngine::Get().GetIoContext()); - auto result (resolver.resolve(node.CStr(), service.CStr())); + auto result (resolver.resolve(node.GetData(), service.GetData())); auto current (result.begin()); for (;;) { @@ -71,7 +71,7 @@ void Connect(Socket& socket, const String& node, const String& service, boost::a using boost::asio::ip::tcp; tcp::resolver resolver (IoEngine::Get().GetIoContext()); - auto result (resolver.async_resolve(node.CStr(), service.CStr(), yc)); + auto result (resolver.async_resolve(node.GetData(), service.GetData(), yc)); auto current (result.begin()); for (;;) { diff --git a/lib/remote/apilistener.cpp b/lib/remote/apilistener.cpp index 6c684e9f1..6bcf5bb5f 100644 --- a/lib/remote/apilistener.cpp +++ b/lib/remote/apilistener.cpp @@ -439,7 +439,7 @@ bool ApiListener::AddListener(const String& node, const String& service) try { tcp::resolver resolver (io); - auto result (resolver.resolve(node.CStr(), service.CStr(), tcp::resolver::passive)); + auto result (resolver.resolve(node.GetData(), service.GetData(), tcp::resolver::passive)); auto current (result.begin()); for (;;) { From d1d399f8b329dbe3f6f1f172ece165332fb9512f Mon Sep 17 00:00:00 2001 From: Julian Brost Date: Fri, 11 Apr 2025 13:05:21 +0200 Subject: [PATCH 7/8] Avoid multiple #if in a single function call expression Simply giving two entire call expressions for either Boost version greatly improves readability in my opinion. --- lib/base/io-engine.hpp | 43 ++++++++++++++++++++---------------------- 1 file changed, 20 insertions(+), 23 deletions(-) diff --git a/lib/base/io-engine.hpp b/lib/base/io-engine.hpp index 2083b62f8..2d2c5e922 100644 --- a/lib/base/io-engine.hpp +++ b/lib/base/io-engine.hpp @@ -105,35 +105,32 @@ public: template static void SpawnCoroutine(Handler& h, Function f) { - - boost::asio::spawn(h, -#if BOOST_VERSION >= 108700 - std::allocator_arg, - boost::context::fixedsize_stack(GetCoroutineStackSize()), -#endif // BOOST_VERSION >= 108700 - [f](boost::asio::yield_context yc) { - + auto wrapper = [f](boost::asio::yield_context yc) { + try { + f(yc); + } catch (const std::exception& ex) { + Log(LogCritical, "IoEngine") << "Exception in coroutine: " << DiagnosticInformation(ex); + } catch (...) { try { - f(yc); - } catch (const std::exception& ex) { - Log(LogCritical, "IoEngine") << "Exception in coroutine: " << DiagnosticInformation(ex); + Log(LogCritical, "IoEngine", "Exception in coroutine!"); } catch (...) { - try { - Log(LogCritical, "IoEngine", "Exception in coroutine!"); - } catch (...) { - } - - // Required for proper stack unwinding when coroutines are destroyed. - // https://github.com/boostorg/coroutine/issues/39 - throw; } - }, + + // Required for proper stack unwinding when coroutines are destroyed. + // https://github.com/boostorg/coroutine/issues/39 + throw; + } + }; + #if BOOST_VERSION >= 108700 + boost::asio::spawn(h, + std::allocator_arg, boost::context::fixedsize_stack(GetCoroutineStackSize()), + std::move(wrapper), boost::asio::detached -#else // BOOST_VERSION >= 108700 - boost::coroutines::attributes(GetCoroutineStackSize()) // Set a pre-defined stack size. -#endif // BOOST_VERSION >= 108700 ); +#else // BOOST_VERSION >= 108700 + boost::asio::spawn(h, std::move(wrapper), boost::coroutines::attributes(GetCoroutineStackSize())); +#endif // BOOST_VERSION >= 108700 } static inline From d3fae440d4b229581c39bc972ea2f3edf0d23efe Mon Sep 17 00:00:00 2001 From: Julian Brost Date: Tue, 15 Apr 2025 15:10:12 +0200 Subject: [PATCH 8/8] SpawnCoroutine: move callback into wrapper lambda MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit f isn't used otherwise in the function, so if possible, it can just be moved into the lambda, avoiding a copy. Co-authored-by: Alexander Aleksandrovič Klimov --- lib/base/io-engine.hpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/base/io-engine.hpp b/lib/base/io-engine.hpp index 2d2c5e922..64831ff8c 100644 --- a/lib/base/io-engine.hpp +++ b/lib/base/io-engine.hpp @@ -105,7 +105,7 @@ public: template static void SpawnCoroutine(Handler& h, Function f) { - auto wrapper = [f](boost::asio::yield_context yc) { + auto wrapper = [f = std::move(f)](boost::asio::yield_context yc) { try { f(yc); } catch (const std::exception& ex) {