Thread-safety of cursors

[tusooa]

I have been thinking that cursors are thread-safe, but it seems not the case.

#include <lager/store.hpp>
#include <vector>
#include <boost/asio.hpp>
#include <lager/event_loop/boost_asio.hpp>
#include <random>
#include <iostream>

using Model = std::vector<int>;
struct Action {};

const int len = 3000;

Model update(Model, Action)
{
    auto r = std::rand() %3;
    return Model(len, r);
}

void print(Model m)
{
    for (auto s : m) {
        std::cout << s;
    }
    std::cout << std::endl;
}

int main()
{
    boost::asio::io_context io;

    auto store = lager::make_store<Action>(
        Model(len, 0),
        lager::with_boost_asio_event_loop{io.get_executor()}
        );

    auto reader = lager::reader<Model>(store);

    store.dispatch(Action{});

    boost::asio::executor_work_guard g(io.get_executor());

    std::thread([&io] { io.run(); }).detach();

    std::thread([&store] {
                    std::this_thread::sleep_for(std::chrono::milliseconds(1));
                    for (auto i = 0; i < 500; ++i) {
                        std::this_thread::sleep_for(std::chrono::microseconds(39));
                        store.dispatch(Action{});
                    }
                }).detach();



    for (auto i = 0; i < 700; ++i) {
        std::this_thread::sleep_for(std::chrono::microseconds(40));
        print(reader.get());
    }
}

Using this code to execute:

./readertest | perl -ne '$len = 3000;chomp; if ($_ ne "0" x $len and $_ ne "1" x $len and $_ ne "2" x $len) { print;print "\n" }'

If cursors are thread-safe, readertest will always print out lines with 3000 same digits of 0 or 1 or 2, so the pipe should print out nothing. But in fact it occasionally print out lines like

111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111122222222222222222222222222222222222222222222222222222222111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111

(the sequence is mostly 1 but contains some 2 in the middle) which means reader.get() gives us corrupted data.

Does this mean we should not use cursors outside the thread that contains the event loop? In particular, the following operations:

0. Construct a cursor by deriving from a local cursor object (In the program, calling reader.map(...).make() in the main thread)
1. Calling get() on a local cursor object (reader.get() in the main thread)
2. Construct a cursor and use it only in another thread (std::thread([reader2=reader] { reader2.get(); }).detach();)

Which of these are safe, considering the event loop is not running on the main thread?

If any of these are not safe, how can we mitigate the thread-unsafety here?

Thank you very much for your help.

[arximboldi]

No they are not thread-safe!

• context::dispatch() is thread-safe (it depends on the main loop implementation, but it is so for with_boost_asio_event_loop).
• reader::get() must happen in the thread asociated to the event loop! In this case, it must happen in this thread once it's launched:

     std::thread([&io] { io.run(); }).detach();

  Note that watchers will be called in that thread anyway.
• All reader tree manipulations (creating new "derived" readers) must happen in the same thread too!

Mitigations depend on what you are trying to achieve. One option to have a cursor tree in another thread is to have a second cursor root (a lager::store or lager::state), watch the data in the original cursor, then pass the data to the other thread and set it in the second store/state.

For example:

    // this is the first store
    auto io1 = boost::asio::io_context{};
    auto store1 = lager::make_store<Action>(
        Model(...),
        lager::with_boost_asio_event_loop{io1.get_executor()});

    // a secondary store just to have data consumed in another thread
    auto io2 = boost::asio::io_context{};
    auto store2 = lager::make_store<Model>(
        Model(...),
        lager::with_boost_asio_event_loop{io1.get_executor()},
        lager::with_reducer([](auto&&, auto m) { return m; }));

    // send data to second store when it changes in first store
    store1.watch([&](auto m) { store2.dispatch(m); });

    // each store has its own thread...
    std::thread([&io1] { io1.run(); }).detach();
    std::thread([&io2] { io2.run(); }).detach();

This is an example with boost::asio. In other cases you may be able to things in other ways if you have for example libdispatch or Qt at hand, and maybe even just use lager::state for the secondary store.

[tusooa]

Thank you, I'll try.