at and operator[] methods return a reference to the value in the map with the map unlocked

[sldr]

This assumes the operator[] methods get fixed with the auto&. If fixed some other way, then at methods are broken.

Allowing a reference to the value in the map out without map locked means erase can be called that would delete the value in the map that you still have the reference to and trying to use it after erase would be a huge problem.

Example code showing problem:

  tsmap<std::string, std::string> mytsmap;
  mytsmap["asdf"] = "LONGstringLONGstringLONGstringLONGstringLONGstringLONGstringLONGstringLONGstringLONGstringLONGstring";
  auto& refinmytsmap = mytsmap.at("asdf");
  mytsmap.erase("asdf");
  auto badptr = refinmytsmap.c_str(); // BAD

[DenisM3lo]

This is a hard one. I'll keep to @tsixta decide.

One simple solution would be removing the references and returning copies.

mapped_type& at (const key_type& k);
const mapped_type& at (const key_type& k) const;

to:

mapped_type at (const key_type& k);
mapped_type at (const key_type& k) const;

Another elegant one would require a return wrapper class to keep the read lock until the reference isn't necessary anymore. Something like:

#include <functional>

template <typename T>
class RetWrapper
{
    T& value;
    std::function<void()> on_destroy;
public:
    RetWrapper(T& value_, std::function<void()> on_destroy_)
        : value(value_), on_destroy(on_destroy_)
    {
    }

    virtual ~RetWrapper()
    {
        if (on_destroy) {
            on_destroy();
        }
    }

    T& get()
    {
        return value;
    }

    T& operator*()
    {
        return value;
    }

    T* operator->()
    {
        return &value;
    }
};

and using:

mapped_type& at(const key_type& k)
{
    this->rwl.lock_read();
    auto& ret = this->std::map<Key, T, Compare, Alloc>::at(k);
    // return the wrapper to keep the lock until the reference isn't necessary anymore.
    return *RetWrapper<decltype(ret)>(ret, [this]() { this->rwl.unlock_read(); });
}

const mapped_type& at(const key_type& k) const
{
    this->rwl.lock_read();
    auto& ret = this->std::map<Key, T, Compare, Alloc>::at(k);
    // return the wrapper to keep the lock until the reference isn't necessary anymore.
    return *RetWrapper<decltype(ret)>(ret, [this]() { this->rwl.unlock_read(); });
}

[DenisM3lo]

This is a hard one. I'll keep to @tsixta decide.

One simple solution would be removing the references and returning copies.

mapped_type& at (const key_type& k);
const mapped_type& at (const key_type& k) const;

to:

mapped_type at (const key_type& k);
mapped_type at (const key_type& k) const;

Another elegant one would require a return wrapper class to keep the read lock until the reference isn't necessary anymore. Something like:

#include <functional>

template <typename T>
class RetWrapper
{
    T& value;
    std::function<void()> on_destroy;
public:
    RetWrapper(T& value_, std::function<void()> on_destroy_)
        : value(value_), on_destroy(on_destroy_)
    {
    }

    virtual ~RetWrapper()
    {
        if (on_destroy) {
            on_destroy();
        }
    }

    T& get()
    {
        return value;
    }

    T& operator*()
    {
        return value;
    }

    T* operator->()
    {
        return &value;
    }
};

and using:

mapped_type& at(const key_type& k)
{
    this->rwl.lock_read();
    auto& ret = this->std::map<Key, T, Compare, Alloc>::at(k);
    // return the wrapper to keep the lock until the reference isn't necessary anymore.
    return *RetWrapper<decltype(ret)>(ret, [this]() { this->rwl.unlock_read(); });
}

const mapped_type& at(const key_type& k) const
{
    this->rwl.lock_read();
    auto& ret = this->std::map<Key, T, Compare, Alloc>::at(k);
    // return the wrapper to keep the lock until the reference isn't necessary anymore.
    return *RetWrapper<decltype(ret)>(ret, [this]() { this->rwl.unlock_read(); });
}

To point out about the second approach: the limitations would be a little overhead by creating the wrapper object; and using only with short living objects, temporarily, otherwise it may cause bugs like endless locks.

[tsixta]

Hi all,

I appreciate your interest in the project. I'm currently not able to meaningfully respond to the issues and pull requests, I have lots of stuff happening in the real life and don't have capacity to thoroughly think through the consequences of any of the changes. I'll keep the code simple (albeit imperfect), but feel free to fork the repo, I'm glad to see that this old pet project attracts some attention. Just if you make the fork public, please put in the readme a link that it originates from here.

Tomas