3

The following example completes with no assertions:

#include <cassert>
#include <functional>
#include <future>
#include <thread>
#include <boost/asio.hpp>

class example1
{
public:
    typedef boost::asio::io_context io_context;
    typedef boost::asio::io_context::executor_type executor_type;
    typedef boost::asio::strand<executor_type> strand;
    typedef boost::asio::executor_work_guard<executor_type> work_guard;
    typedef std::function<void()> handler;

    example1()
      : work_(boost::asio::make_work_guard(context_)),
        thread_([this]() { context_.run(); }),
        strand1_(context_.get_executor()),
        strand2_(context_.get_executor())
    {

    }

    ~example1()
    {
        assert(result_.get_future().get());
        work_.reset();
        thread_.join();
    }

    void invoke()
    {
        handler handle = boost::asio::bind_executor(strand2_,
            std::bind(&example1::strand2_handler, this));

        boost::asio::post(strand1_,
            std::bind(&example1::strand1_handler, this, handle));
    }

    void strand1_handler(handler handle)
    {
        assert(strand1_.running_in_this_thread());
        handle();
    }

    void strand2_handler()
    {
        assert(strand1_.running_in_this_thread());
        ////assert(strand2_.running_in_this_thread());
        result_.set_value(true);
    }

private:
    io_context context_;
    work_guard work_;
    std::thread thread_;
    strand strand1_;
    strand strand2_;
    std::promise<bool> result_;
};

int main()
{
    example1 test{};
    test.invoke();
}

However my expectation is that the commented-out assertion should succeed, as opposed to the one directly above it. According to strand::running_in_this_thread() the handler handle has been invoked in the caller's strand, not that provided to bind_executor.

I can work around this using intermediate methods, as follows.

class example2
{
public:
    typedef boost::asio::io_context io_context;
    typedef boost::asio::io_context::executor_type executor_type;
    typedef boost::asio::strand<executor_type> strand;
    typedef boost::asio::executor_work_guard<executor_type> work_guard;
    typedef std::function<void()> handler;

    example2()
      : work_(boost::asio::make_work_guard(context_)),
        thread_([this]() { context_.run(); }),
        strand1_(context_.get_executor()),
        strand2_(context_.get_executor())
    {

    }

    ~example2()
    {
        assert(result_.get_future().get());
        work_.reset();
        thread_.join();
    }

    void invoke()
    {
        handler handle =
            std::bind(&example2::do_strand2_handler, this);

        boost::asio::post(strand1_,
            std::bind(&example2::strand1_handler, this, handle));
    }

    void strand1_handler(handler handle)
    {
        assert(strand1_.running_in_this_thread());
        handle();
    }

    // Do the job of bind_executor.
    void do_strand2_handler()
    {
        boost::asio::post(strand2_,
            std::bind(&example2::strand2_handler, this));
    }

    void strand2_handler()
    {
        ////assert(strand1_.running_in_this_thread());
        assert(strand2_.running_in_this_thread());
        result_.set_value(true);
    }

private:
    io_context context_;
    work_guard work_;
    std::thread thread_;
    strand strand1_;
    strand strand2_;
    std::promise<bool> result_;
};

int main()
{
    example2 test2{};
    test2.invoke();
}

But avoiding that is presumably the purpose of bind_executor. Is this a boost bug or am I missing something? I've tried following this through the boost::asio sources but to no avail.

Update

Thanks to @sehe for a lot of help. The above problem can be resolved in a number of ways, for example:

class example3
{
public:
    typedef boost::asio::io_context io_context;
    typedef boost::asio::io_context::executor_type executor_type;
    typedef boost::asio::strand<executor_type> strand;
    typedef boost::asio::executor_work_guard<executor_type> work_guard;
    typedef boost::asio::executor_binder<std::function<void()>,
        boost::asio::any_io_executor> handler;

    example3()
      : work_(boost::asio::make_work_guard(context_)),
        thread_([this]() { context_.run(); }),
        strand1_(context_.get_executor()),
        strand2_(context_.get_executor())
    {
    }

    ~example3()
    {
        assert(result_.get_future().get());
        work_.reset();
        thread_.join();
    }

    void invoke()
    {
        auto handle = boost::asio::bind_executor(strand2_,
            std::bind(&example3::strand2_handler, this));

        boost::asio::post(strand1_,
            std::bind(&example3::strand1_handler, this, handle));
    }

    void strand1_handler(handler handle)
    {
        assert(strand1_.running_in_this_thread());
        boost::asio::dispatch(handle);
    }

    void strand2_handler()
    {
        assert(strand2_.running_in_this_thread());
        result_.set_value(true);
    }

private:
    io_context context_;
    work_guard work_;
    std::thread thread_;
    strand strand1_;
    strand strand2_;
    std::promise<bool> result_;
};

int main
{
    example3 test3{};
    test3.invoke();
}
evoskuil
  • 1,011
  • 1
  • 7
  • 13
  • 1
    I noticed your edit. Indeed, the declaration/initialization order was not okay. The sample still doesn't compile (e.g. the `example` vs `example1` name conflict). If you really want a class like that (instead of how I lay it out with lambdas in my answer), I'd write it like this: http://coliru.stacked-crooked.com/a/2e4c463d036c93e0. Note the subtle fix of ordering the last assert _after_ the join. – sehe Mar 15 '22 at 01:34
  • The problem with manually editing here :/. – evoskuil Mar 15 '22 at 02:47
  • I saw the comment on the promise order, though I don't see how it's consequential. The intent is to keep the threadpool alive until the promise is set, which this makes explicit. – evoskuil Mar 15 '22 at 02:48
  • Agree, the layout of the example class isn't ideal, some nice edits. – evoskuil Mar 15 '22 at 02:52
  • 1
    Ah, yeah `get()` blocks anyways. In my mind I read it as a `is_ready(future)` check - which conceptually is the whole purpose of the assert. In that case, the order would matter, to avoid a race condition (not a data race). (Sidenote: The thread_pool doesn't _have_ to remain for the future to be valid, so my reorder was ok) – sehe Mar 15 '22 at 02:58
  • Agreed, since the call to invoke is necessary for the class not to hang on destruct, ordering doesn't actually matter. – evoskuil Mar 15 '22 at 03:35

1 Answers1

8

Yes, you're missing something indeed. Two things, actually.

Type Erasure

Binding an executor doesn't modify the function, it modifies its type.

However, by erasing the callable's type using std::function<> you've hidden the bound executor. You could easily determine this:

erased_handler handle = bind_executor(s2, s2_handler);
assert(asio::get_associated_executor(handle, s1) == s1);

The problem is gone when you preserve the type:

auto handle = bind_executor(s2, s2_handler);
assert(asio::get_associated_executor(handle, s1) == s2);

Dispatch (formerly handler_invoke)

Invoking handle straight up calls it according to the C++ language semantics, as you have found out.

To ask Asio to honour the potentially bound executor, you could use dispatch (or post):

auto s1_handler = [&](auto chain) {
    assert(s1.running_in_this_thread());
    dispatch(get_associated_executor(chain, s1), chain);
};

In fact, if you're sure that chain will have an associated executor, you could accept the default fallback (which is a system executor):

auto s1_handler = [&](auto chain) {
    assert(s1.running_in_this_thread());
    dispatch(chain);
};

Putting It All Together

Demonstrating the wisdom in a simplified, extended tester:

Live On Coliru

#include <boost/asio.hpp>
#include <functional>
#include <iostream>

namespace asio = boost::asio;

int main() {
    asio::thread_pool io(1);

    auto s1 = make_strand(io), s2 = make_strand(io);
    assert(s1 != s2); // implementation defined! strands may hash equal

    auto s1_handler = [&](auto chain) {
        assert(s1.running_in_this_thread());

        // immediate invocation runs on the current strand:
        chain();

        // dispatch *might* invoke directly if already on the right strand
        dispatch(chain);                                     // 1
        dispatch(get_associated_executor(chain, s1), chain); // 2

        // posting never immediately invokes, even if already on the right
        // strand
        post(chain);                                     // 3
        post(get_associated_executor(chain, s1), chain); // 4
    };

    int count_chain_invocations = 0;
    auto s2_handler = [&] {
        if (s2.running_in_this_thread()) {
            count_chain_invocations += 1;
        } else {
            std::cout << "(note: direct C++ call ends up on wrong strand)\n";
        }
    };

    {
        using erased_handler  = std::function<void()>;
        erased_handler handle = bind_executor(s2, s2_handler);
        assert(asio::get_associated_executor(handle, s1) == s1);
    }
    {
        auto handle = bind_executor(s2, s2_handler);
        assert(asio::get_associated_executor(handle, s1) == s2);
    }

    auto handle = bind_executor(s2, s2_handler);
    post(s1, std::bind(s1_handler, handle));

    io.join();

    std::cout << "count_chain_invocations: " << count_chain_invocations << "\n";
}

All the assertions pass, and the output is as expected:

(note: direct C++ call ends up on wrong strand)
count_chain_invocations: 4

BONUS: What If You Need Type-Erased Bound Calleables?

Whatever you do, don't use std::function. You can wrap one, though;

template <typename Sig> struct ErasedHandler {
    using executor_type = asio::any_io_executor;
    std::function<Sig> _erased;
    executor_type      _ex;
    executor_type get_executor() const { return _ex; }

    template <typename F>
    explicit ErasedHandler(F&& f)
        : _erased(std::forward<F>(f))
        , _ex(asio::get_associated_executor(f)) {}

    ErasedHandler() = default;

    template <typename... Args>
    decltype(auto) operator()(Args&&... args) const {
        return _erased(std::forward<Args>(args)...);
    }
    template <typename... Args>
    decltype(auto) operator()(Args&&... args) {
        return _erased(std::forward<Args>(args)...);
    }

    explicit operator bool() const { return _erased; }
};

See it Live On Coliru

Before you do, note that

  • using any_io_executor also type erases the executor, which potentially hurts performance
  • it does not provide a good fallback, just using the system executor for unbound calleables. You could get around this by detecting it and requiring an explicit constructor arugment etc. but...
  • all of this still completely ignores other handler attributes like associated allocator

I would probably avoid generically storing type-erased chainable handlers. You can most often store the actual type of the handler deduced by template type parameter.

PS: Afterthoughts

What you were perhaps expecting was this behaviour:

template <typename... Args>
decltype(auto) operator()(Args&&... args) const {
    // CAUTION: NOT WHAT YOU WANT
    boost::asio::dispatch(_ex,
                          std::bind(_erased, std::forward<Args>(args)...));
}
template <typename... Args>
decltype(auto) operator()(Args&&... args) {
    // CAUTION: NOT WHAT YOU WANT
    boost::asio::dispatch(_ex,
                          std::bind(_erased, std::forward<Args>(args)...));
}

See that Live On Coliru

Under this regimen even direct C++ calls will "do the right thing".

That seems nice. Until you think about it.

The issue is that handlers cannot be rebound this way. More specifically, if you had a handler that is associated with a "free-threaded" executor, doing bind_executor(strand, f) would have no effect (except slowing down your program), as the f would be obnoxiously dispatching to another executor anyways.

So don't do that :)

sehe
  • 374,641
  • 47
  • 450
  • 633
  • Comments are not for extended discussion; this conversation has been [moved to chat](https://chat.stackoverflow.com/rooms/242945/discussion-on-answer-by-sehe-boostasiobind-executor-does-not-execute-in-stra). – Samuel Liew Mar 15 '22 at 06:27
  • I personally don't like that Asio expects general-purpose networking libraries built on top of it to be header-only. Some of us care about compile times and being able to apply the Pimpl idiom. Asio should provide a type-erased wrapper for handlers which bundle the associated executor. – Emile Cormier Jul 27 '22 at 16:47
  • @EmileCormier But you can? You can pimpl away to your heart's desire. Obviously that implies a choice of completion token (type) but that should be fine choice at the level of a particular library/application. Loads of people do this. – sehe Jul 27 '22 at 17:11
  • The only thing I see is: Asio **chooses** to be uncompromisingly generic and also **encourages** other general-purpose libraries to propagate that flexibility. That's a far cry from requirement or necessity. – sehe Jul 27 '22 at 17:12
  • @sehe, The problem is that I don't want to impose the choice of completion token upon my library's users, while retaining the ability to compile my library (not be header-only). I already know that some of my library's users want to use stackful couroutines, others C++20 couroutines, and others traditional callbacks. If I want to retain the ability to compile my library, I'm forced to implement my own type-erased handler wrapper which I doubt would be as well optimized as one provided by the Asio author. – Emile Cormier Jul 27 '22 at 18:17
  • 1
    By the way, your deep insights into Asio, as well as your detailed answers on SO are invaluable. Thank you. – Emile Cormier Jul 27 '22 at 18:19
  • My feature request for a type-erased handler wrapper: https://github.com/chriskohlhoff/asio/issues/1100 – Emile Cormier Jul 27 '22 at 18:22
  • @EmileCormier Thanks for the kind words. I just mentioned to someone that I think you were the one to make me "discover" the `async_result` mechanism years ago :) So the feeling is mutual. I'm not aware of a type-erased handler wrapper provided by Asio. I agree that it might be relevant for Asio to provide one. I guess Chris just really carefully chooses his focus: his responsibility is the generic interface of Asio, and perhaps he feels that supplying alternative methods risks muddying the waters (which could further harm the already steep learning curve). – sehe Jul 27 '22 at 20:07
  • @EmileCormier regarding "How do we detect whether a handler has an executor bound" (now deleted?) - we can apply the logic used by `get_associated_executor`. So, the extremely lazy-bum approach would just abuse that to do it: https://godbolt.org/z/d788qK91K – sehe Jul 27 '22 at 20:09
  • 1
    Yes, it wasn't clear in the `get_associated_executor` function documentation, but I found in the `associated_executor` class documentation that you can specify a fallback executor (thus me deleting the question). My understanding of `async_result` years ago was flawed, which ended up breaking my library in one of the Asio 1.6x releases. Fortunately things are getting better with the Asio documentation, as well as the community's understanding of it in general. – Emile Cormier Jul 27 '22 at 21:21
  • Agreed. The new Boost Mysql is a testament to that. I also feel with the new executors things have been considerably more usable, and `co_await` tops it off. – sehe Jul 27 '22 at 21:54