boostorg / multi_index

Boost.org multi_index module
http://boost.org/libs/multi_index
45 stars 59 forks source link

[Question] Calling modify(iterator position,Modifier mod) with no key updating from shared lock is safe ? #50

Closed redboltz closed 3 years ago

redboltz commented 3 years ago

Here are a class and a multi_index contaier.

struct foo {
    int key1;
    int key2;
    std::atomic<int> no_key; // thread-safe itself
};

namespace mi = boost::multi_index;
using mi_foo = mi::multi_index_container<
    foo,
    mi::indexed_by<
        mi::ordered_non_unique<
            mi::key<&foo::key1>
        >,
        mi::ordered_non_unique<
            mi::key<&foo::key2>
        >
    >
>;

key1 and key2 are used as index key but no_key doesn't. In order to access the container, I use boost::shared_mutex.

    boost::shared_mutex m;
    mi_foo mf;

When I insert to mf or erase from mf, I use exclusive lock.

    boost::lock_guard<boost::shared_mutex> g{m}; // exclusive lock guard
    mf.emplace(....);

When I want to update no_key, I can't do as follows:

    boost::shared_lock_guard<boost::shared_mutex> g{m}; // shared lock guard
    auto it = mf.begin(); // in actual code, find something
    ++it->no_key;        

It's because it is const_iterator. It is reasonable that if key1 or key2 is overwritten, indexes would be broken.

So I need to do as follows:

    boost::shared_lock_guard<boost::shared_mutex> g{m}; // shared lock guard
    auto it = mf.begin(); // in actual code, find something
    mf.modify(
        it,
        [](auto& e) {
            ++e.no_key;
        }
    );

After mod lambda expression is called, multi_index container can re-calculate indexes.

modify() is called is shared lock guard. That means multiple threads can call modify() concurrently. Is it safe as long as I update only no key members in modify() function from multiple threads ? NOTE no_key itself is thread safe because std::atomic<int> no_key.

I'm not sure after mod function called process is thread safe if keys are not updated.

joaquintides commented 3 years ago

There's two ways to answer this:

redboltz commented 3 years ago

Thank you for prompt response.

As you mentioned, const_cast is the best choice for this situation.

So I will update as follows:

before

    mf.modify(
        it,
        [](auto& e) {
            ++e.no_key;
        }
    );

after

   ++const_cast<std::atomic<int>&>(it->no_key);