On Sun, May 28, 2023 at 11:18 AM Phil Bouchard <boost_at_[hidden]> wrote:
>
>
>
> On 5/28/23 11:13, Jason McKesson via Std-Proposals wrote:
> > On Sun, May 28, 2023 at 11:04 AM Federico Kircheis via Std-Proposals
> > <std-proposals_at_[hidden]> wrote:
> >>
> >> On 28/05/2023 16.55, Phil Bouchard via Std-Proposals wrote:
> >>> This is pretty much what I had in mind:
> >>> https://github.com/philippeb8/std__ts/blob/master/ts.h
> >>>
> >>> Very cost / beneficial.
> >>>
> >>
> >>
> >> One of the issues I have with such classes is that they make other tools
> >> for finding bugs, like sanitizers and valgrind, useless.
> >>
> >> This code is racy, even with the internal mutex:
> >>
> >> if(container.empty()){
> >> container.insert(....)
> >> }
> >>
> >>
> >> but since the data is protected by mutexes and synchronized, not tools
> >> will complain (AFAIK).
> >>
> >> Thus even if such class would be standardized, in most cases it is
> >> useless/it is a not a good primitive for multi-threaded code.
> >
> > What's worse is that "fixing it" is really hard. You'd have to do
> > something like this
> >
> > ```
> > container_lock c(container);
> > if(container.empty())
> > {
> > container.insert(....)
> > }
> > ```
>
> All you'd have to do is:
>
> std::lock_guard<std::mutex> lock(container.mutex());
> if(container.empty())
> {
> container.insert(....)
> }

1: Your example code has no such function.

2: All that would do is provoke UB. `empty` will attempt to lock the
mutex. Locking a `std::mutex` that is already locked by your thread
yields UB. You would need a `recursive_mutex`, which is basically the
thing I described with thread IDs wrapped in its own object. In short:
it adds overhead.