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

Compile error when any_io_executor based websocket stream with bind_executor strand #2775

Open
redboltz opened this issue Dec 15, 2023 · 10 comments

Comments

@redboltz
Copy link

Version of Beast

  • My local:
    • Boost 1.84.0,
    • Beast 351
  • Godbolt
    • Boost 1.83.0
    • Beast 347

Steps necessary to reproduce the problem

Compile the following code:

#include <boost/asio.hpp>
#include <boost/beast/websocket/stream.hpp>

namespace net = boost::asio;
namespace bs = boost::beast;

using tcp = net::basic_stream_socket<net::ip::tcp, net::any_io_executor>;
using ws = bs::websocket::stream<tcp>;

void tcp_test() {
    net::io_context ioc;
    net::any_io_executor exe = ioc.get_executor();

    std::string buf;
    tcp socket{exe};
    auto str = net::make_strand(exe);
    socket.async_write_some(
        net::buffer(buf), 
        net::bind_executor( // on tcp socket with any_io_executor, no error happens
            str,
            [](auto, auto){}
        )
    );
}

void ws_test() {
    net::io_context ioc;
    net::any_io_executor exe = ioc.get_executor();

    std::string buf;
    ws web_socket{exe};
    auto str = net::make_strand(exe);
    web_socket.async_write(
        net::buffer(buf), 
#if 1 // If you set 0 (remove bind_executor), then error is disappeared
        net::bind_executor(
            str,
            [](auto, auto){}
        )
#else
        [](auto, auto){}
#endif
    );
}

int main() {}

Got the following compile error:

explorer/libs/boost_1_83_0/boost/asio.hpp:188:
/opt/compiler-explorer/libs/boost_1_83_0/boost/asio/strand.hpp:260:15: error: no member named 'on_work_finished' in 'boost::asio::any_io_executor'
  260 |     executor_.on_work_finished();
      |     ~~~~~~~~~ ^

godbolt link(x86-64 clang 17.0.1) : https://godbolt.org/z/nocj654EK

All relevant compiler information

  • x86-64 clang 17.0.1
  • x86-64 gcc 13.2

What I noticed

  • tcp doesn't cause the error.
  • The error happens only the combination of ws with bind_executor strand.
    • ws contains tcp as the next layer. tcp has any_io_executor.
    • The strand str is created by make_strand and any_io_executor exe is passed.
@ashtum
Copy link
Collaborator

ashtum commented Dec 15, 2023

The issue is associated with creating a work_guard from a strand<any_io_executor>. It attempts to utilize on_work_started() and on_work_finished() functions, which are not defined for asio::any_io_executor. This results in a compile error in websocket::stream<tcp>::async_write because it attempts to create a work_guard from the associated executor.
It can be reproduced with the following code as well: https://godbolt.org/z/nvr9jjnrY

#include <boost/asio.hpp>

namespace asio = boost::asio;

int main()
{
    asio::io_context ioc;
    asio::any_io_executor exec = ioc.get_executor();
    auto strand = asio::make_strand(exec);
    auto wg = asio::make_work_guard(strand);
}

I will further investigate to determine whether we can address this in Beast or if it is a bug in Asio.

@redboltz
Copy link
Author

I'm not 100% sure but passing strand.get_inner_executor() to make_work_guard could solve the issue ?

#include <boost/asio.hpp>

namespace asio = boost::asio;

int main()
{
    asio::io_context ioc;
    asio::any_io_executor exec = ioc.get_executor();
    auto strand = asio::make_strand(exec);
    auto wg = asio::make_work_guard(strand.get_inner_executor());
}

https://godbolt.org/z/jv1zEW7ET

@ashtum
Copy link
Collaborator

ashtum commented Dec 15, 2023

I'm not 100% sure but passing strand.get_inner_executor() to make_work_guard could solve the issue ?

This will generate a work guard for any_io_executor instead of strand<any_io_executor>. Calling get_executor on this work guard will return an instance of any_io_executor.
Although I believe it will function correctly in this case, as we don't retrieve the executor from the work guard, I am seeking a cleaner solution or addressing this in Asio.

@ashtum
Copy link
Collaborator

ashtum commented Dec 15, 2023

I've raised an issue in Asio for this; let's see what will be the response. chriskohlhoff/asio#1394

redboltz added a commit to redboltz/async_mqtt that referenced this issue Dec 17, 2023
ws and wss don't support any_io_executor due to the following issue:
boostorg/beast#2775
@redboltz
Copy link
Author

chriskohlhoff/asio#1394 still no response. It seems that there are a lot of uncommented issues in the project.
I think that this issue is often appered using C++20 coroutine with strand in the real world.
See https://godbolt.org/z/7EnWa3Es1
If there were no update until the next release, I would appliciate if you implemented some workaround on the beast side .

@ashtum
Copy link
Collaborator

ashtum commented Jan 11, 2024

Yeah, Chris usually doesn't respond to the issues, but he reads them. We should track the changes in the repository to see if it gets fixed or not.

@redboltz
Copy link
Author

Boost 1.85.0 has been released. Any updates about the issue?

@ashtum
Copy link
Collaborator

ashtum commented Apr 16, 2024

No, unfortunately, this issue, along with many others, hasn't been addressed in the Asio for 1.85.0 release.

@redboltz
Copy link
Author

I noticed that I used strand in an unusual way; my library no longer uses strand like that. Here is another proof-of-concept (PoC) code example based on the code I initially reported:

https://godbolt.org/z/1q6vxzfja

        net::bind_executor(
            net::any_io_executor{str}, // no error, strand still effects
            [](auto, auto){}
        )

My issue has been solved, but I leave it up to you whether to close the issue or not.

@ashtum
Copy link
Collaborator

ashtum commented May 22, 2024

Thanks for reporting this.

The root cause of the issue is creating a work_guard from a strand<any_io_executor> because Asio chose the wrong specialization. Yes, if we wrap that strand in an any_completion_executor or any_io_executor, the problem disappears because now we are creating a work_guard from these types, not the strand itself. The downside is it creates an extra layer of indirection, which I would say is insignificant in practice.

I would suggest keeping this issue open as this is more of a workaround and the root cause still exists.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants