Compatibility fixes for Boost 1.87 #1164
Conversation
…ated now-removed functionality
…improved time-to-expiry calculation (credit toonetown), added CMake flag to compile tests and examples with ASIO standalone, fixed example and test compilation with ASIO standalone
|
I've made minor modifications to the build script as well as to the examples and tests to allow me to compile them with ASIO standalone and I can confirm these changes do not break ASIO standalone: they are compatible with both backend options. |
There was a problem hiding this comment.
Please note that all of these are merely suggestions - this patch is extremely useful as-is and I don't see any "blocking" issues with it. The suggestions I provide are just based off of reconciliation with the patch work that I did and mainly revolve around naming differences and conventions.
Additionally (outside the individual suggestions below), there is also one reference remaining to io_service in
Line 19 in b9aeec6
| @@ -77,6 +77,7 @@ include (CMakeHelpers) | |||
| option (ENABLE_CPP11 "Build websocketpp with CPP11 features enabled." TRUE) | |||
| option (BUILD_EXAMPLES "Build websocketpp examples." FALSE) | |||
| option (BUILD_TESTS "Build websocketpp tests." FALSE) | |||
| option (USE_ASIO_STANDALONE "Build websocketpp examples and tests using the standalone ASIO library." FALSE) | |||
There was a problem hiding this comment.
I love that you added the ability to test against Asio standalone as well. I didn't think of doing that!
| @@ -240,6 +240,7 @@ class connection : public lib::enable_shared_from_this<connection> { | |||
|
|
|||
| // run the hostname through make_address to check if it is a valid IP literal | |||
| lib::asio::ip::address addr = lib::asio::ip::make_address(host, ec_addr); | |||
| (void)addr; | |||
There was a problem hiding this comment.
I almost fixed this in mine as well...the warning was BUGGING me! 😄
| // strictly necessary, but simplifies thread management a bit. | ||
| boost::asio::io_service ios; | ||
| websocketpp::lib::asio::io_context ios; |
There was a problem hiding this comment.
I'd suggest changing the name of this variable to ctx instead of ios to reflect that it's a context not a service.
test/endpoint/endpoint.cpp
Outdated
| @@ -53,7 +53,7 @@ BOOST_AUTO_TEST_CASE( initialize_server_asio ) { | |||
|
|
|||
| BOOST_AUTO_TEST_CASE( initialize_server_asio_external ) { | |||
| websocketpp::server<websocketpp::config::asio> s; | |||
| boost::asio::io_service ios; | |||
| websocketpp::lib::asio::io_context ios; | |||
There was a problem hiding this comment.
Suggest changing variable name to ctx instead of ios.
test/transport/asio/timers.cpp
Outdated
| @@ -79,7 +79,7 @@ void run_dummy_server(int port) { | |||
|
|
|||
| // Wait for the specified time period then fail the test | |||
| void run_test_timer(long value) { | |||
| boost::asio::io_service ios; | |||
| boost::asio::io_context ios; | |||
There was a problem hiding this comment.
Suggest renaming this variable to ctx instead of ios
| @@ -714,7 +712,7 @@ class endpoint : public config::socket_type { | |||
| * @since 0.3.0 | |||
| */ | |||
| void start_perpetual() { | |||
| m_work.reset(new lib::asio::io_service::work(*m_io_service)); | |||
| m_work.reset(new lib::asio::executor_work_guard<lib::asio::io_context::executor_type>(m_io_context->get_executor())); | |||
There was a problem hiding this comment.
I actually suggest renaming the member variable as m_work_guard instead of m_work.
| tcp::resolver::iterator end; | ||
| if (endpoint_iterator == end) { | ||
| tcp::resolver r(*m_io_context); | ||
| tcp::resolver::results_type endpoints = r.resolve(host, service); |
There was a problem hiding this comment.
Suggest naming the variable something like results instead of endpoints - as they are of type results_type (and "results" seems to be the asio/boost way to refer to them)
There was a problem hiding this comment.
Additionally, I'd suggest updating the comment for this function since it refers to "Asio documentation for ip::basic_resolver_query::basic_resovler_query's constructors"...which we aren't using. I'd direct them instead to the "Asio documentation for the ip::basic_resolver::resolve function". There are two places in comments where it references the basic_resolver_query constructor (the two listen functions)
| m_elog->write(log::elevel::library, | ||
| "asio::listen could not resolve the supplied host or service"); | ||
| ec = make_error_code(error::invalid_host_service); | ||
| return; | ||
| } | ||
| listen(*endpoint_iterator,ec); | ||
| listen(*endpoints.begin(),ec); |
There was a problem hiding this comment.
Personal preference (for readability) is to use parenthesis when dereferencing like this so it is clear to which it is applying. Something like this:
| listen(*endpoints.begin(),ec); | |
| listen(*(endpoints.begin()),ec); |
| void reset() { | ||
| m_io_service->reset(); | ||
| m_io_context->restart(); |
There was a problem hiding this comment.
I also suggest renaming this function to restart to match the fact that it's just passing through to the underlying io_context object. If you make this change, you ALSO need to change what is called in
| @@ -995,10 +993,10 @@ class endpoint : public config::socket_type { | |||
|
|
|||
| void handle_resolve(transport_con_ptr tcon, timer_ptr dns_timer, | |||
| connect_handler callback, lib::asio::error_code const & ec, | |||
| lib::asio::ip::tcp::resolver::iterator iterator) | |||
| lib::asio::ip::tcp::resolver::results_type endpoints) | |||
There was a problem hiding this comment.
Suggest renaming the parameter as results to match results_type instead of endpoints
|
@toonetown Thanks for the feedback, I've implemented almost all of your suggestions! I left two out:
Let me know if there's any other work that needs doing. |
|
@amini-allight In order to check for expected future problems I tried to build with BOOST_ASIO_NO_DEPRECATED defined, Any chance to fix the remaining references on deprecated asio features? |
I certainly can, if you could detail what errors you've experienced that would be a big help. I tried compiling the examples and tests with Have you experienced other errors besides these when using |
|
problems I see in my project are about strand::wrap |
…io::bind_executor to avoid future deprecation issues
|
@Kinokin I've added a new commit which addresses this. Let me know if you find any other deprecations which need fixing. |
|
@amini-allight Most failures in my test application went away and I managed to fix the last one. Again, thanks a lot! |
Some compatibility fixes for Boost 1.87. This commit should be reverted once after zaphoyd/websocketpp#1164 is available.
|
Thanks for these fixes! Added it to our application, compiles and works just fine. |
|
@amini-allight should the version check for boost in CMakeLists.txt not have its version increased from 1.39.0 and a version check added to SConstruct? Similarly if the minimum version of boost is now above 1.43.0 then should https://github.com/zaphoyd/websocketpp/blob/master/websocketpp/common/random.hpp#L58 be changed? |
… in CMakeLists, bumped CMakeLists version, removed unnecessary boost compatibility macros
|
@loqs Hi, sorry for the delay. I've bumped the version in the I didn't change the |
|
I assume this change removes compatibility with the older versions of Asio? |
|
@zaphoyd - yeah - will break if using versions of asio that are more than 8 years old. Versions that old are very unlikely to be encountered in the wild…and this change is required to support current asio and boost. There has been an 8-year deprecation time window - seems a change like this is just in keeping up with the times. If you have a requirement to use 8-year-old asio, you can use older websocket++ as well. |
|
Noted. As I review I am trying to assess whether some sort of fallback is needed. I think I agree that at this point falling back to an older version of WebSocket++ is likely to be the best solution for that narrow case. Also, thanks for your (and others here) work on this. Going to try and get this reviewed and packaged up into an official release. |
|
@zaphoyd To expand on what toonetown said compatibility with boost 1.66.0 and onwards should be preserved but I have not tested this extensively (I can do so if you like). Any version before that will not work because, at a minimum, The CMake version bump probably also introduces incompatibility with some old CMake versions but I'm not sure it's possible to win with CMake: an older specified version causes incompatibility with newer executables just as much as a newer specified version causes incompatibility with older executables. Great to hear you're looking to merge this! |
|
@zaphoyd Are there any plans to merge this pull request? |
On December 12th 2024 Boost 1.87 was released. This release finally removed support for a number of pieces of Boost.ASIO functionality that had been deprecated since roughly Boost 1.66 back in 2017. Most importantly
boost::asio::io_serviceis no longer available, replaced byboost::asio::io_context. This version of Boost was pushed into the Arch Linux repositories on January 19th 2025 and, once I updated my system, caused builds to fail in a project of mine that depends upon WebSocket++.I investigated the cause of these failed builds and found that WebSocket++ makes heavy use of this long-deprecated Boost.ASIO functionality, so I authored changes for my own use that rectify this problem. I am unsure if you will want to accept this pull request because the changes are numerous and wide-ranging but I thought I should bring it to your attention. As the developer of a dependent project on a bleeding-edge rolling-release distro I am probably one of the first people to be impacted by this, but the impact will spread as Boost 1.87 begins to be included in regular releases of other distros.
I have tested that this code allows my project to compile, all examples and tests compile and it passes all existing tests. I have not tested whether or not this impacts compatibility with the standalone ASIO backend.
I also have a very similar version of these changes that fix this issue for the master branch which I can make a pull request for upon request.