On 29/06/2023 00.37, Frederick Virchanza Gotham via Std-Proposals wrote:
> A few months ago I was thinking of submitting this to Boost, but now
> I'm actually thinking maybe it should be in the Standard Library.
>
> I'm not the first person to come up with a class like this, but my own
> implementation is one of very few full implementations I've been able
> to find online.
>
> You define a global object as follows:
>
> Reservable< std::vector<int> > g_vec;
>
> From this point forward, multiple threads can access the object safely
> as follows:
>
> g_vec->push_back(42); // This is thread-safe
>
> Or you can do:
>
> auto mylock = g->vec.Reserve();
> mylock->push_back(44);
> mylock->push_back(47);
>
> Or if you want an L-value reference:
>
> auto mylock = g->vec.Reserve();
> auto &vec = *mylock;
> vec.push_back(44);
> vec.push_back(47);
>

I see two issues with this interface

In

----
push_back(44);
push_back(47);
----
you are loking at every push_back, not ideal, and while looking at the 
code it is not obvious.
Also you might have wanted 47 appearing in your vector after 44, since 
vector is "thread-safe", it is not an unfair assumption, but it is not 
necessarily true, as someone else might be pushing something in the 
meantime, and thus introduce a value between 44 and 47.
With
----
>      auto mylock = g->vec.Reserve();
>      auto &vec = *mylock;
----
you avoid the issues mentioned earlier, but the scope of the lock is 
generally too big, and it is easy to oversee it, the code should look 
more like
----
// ...
{
       auto mylock = g->vec.Reserve();
       auto &vec = *mylock;
       vec.push_back(44);
       vec.push_back(47);
}
// ...
----
and I know most people will forget it.
Also it "leaks" the internal object, for example
----
auto vec& = *(g->vec.Reserve());
// bypassing mutex and still access to internal data
----
and also with
----
int& j = vec->at(0);
----
I think such classes are doomed, because hiding the fact there is a 
synchronisation mechanism is rarely a good idea for low-level 
operations/generic classes.
Also operator-> generally still leads to race conditions, it only helps 
to avoid (some) data races
Which means that useful tools like sanitizers and valgrind cannot be 
used anymore.
I believe a better approach is to provide a callback mechanism, with an 
explicit lock function, for example
(https://fekir.info/post/sharing-data-between-threads/#_bind-the-data-and-mutex-together)
----
#include <mutex>
#include <type_traits>
template<class T, class mutex = std::mutex>
class mutexed_obj {
  T obj;
  mutable mutex m;
 public:
  template<typename... Args>
  explicit mutexed_obj(Args&&... args) : obj(std::forward<Args>(args)...) {}
  template<class F>
  friend decltype(auto) lock(const mutexed_obj<T, mutex>& mo, F f){
   std::scoped_lock l(mo.m);
   return f(mo.obj);
  }
  template<class F>
  friend decltype(auto) lock(mutexed_obj<T, mutex>& mo, F f){
   std::scoped_lock l(mo.m);
   return f(mo.obj);
  }
};
----
usage:
mutexed_obj<int> data(42);
lock(data, [](int i){ std::cout << i;});
lock(data, [](int& i){ ++i;});
compared to your proposal:
1) you have always a given scope for a lock, making it easy to make it 
minimal at every invocation
2) you can still leak an object, for example with
int& j = lock(data, [](int& i){ return i;});
int& j = 0;
lock(data, [&](int& i){ j=i });
but it is hopefully easier to verify that there are no leaks.
First, because there is always a "lock" function, which should be a 
warning signal for most developer.
Second, because the class does not try to look like a normal class and 
overload "operator->" and give internal access to the data to the 
caller, but only though a callback.
Third, because thanks to the callback, it is harder to forget to make 
the scope of the locked mutex too broad.
Also you can choose the mutex type, I would prefer not to have 
recursive_mutex by default.
PS: I think there is a similar classes in a paper, I'm not able to find 
it right now.