You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@trafficserver.apache.org by Walt Karas <wk...@verizonmedia.com.INVALID> on 2019/03/20 16:08:33 UTC

Proposed addition to tsutil: Generic mutex-locking reference template

https://godbolt.org/z/6klEJn

Useful for mutex-protected objects, where the mutex is only locked within
code blocks.  Could be used in this PR:

https://github.com/apache/trafficserver/pull/5187

Re: Proposed addition to tsutil: Generic mutex-locking reference template

Posted by Walt Karas <wk...@verizonmedia.com.INVALID>.
The ATS code uses that terminology.  The C++ Standard lib has
std::lock_guard .  If you are saying we should not transition to the stdlib
stuff, why?

The utility I'm proposing makes it a little more convenient to use a mutex
with lock_guards in typical cases that a mutex is used.  It also makes it
very clear what the mutex is protecting, and makes accessing the protected
data less error prone.  To me, it's a big problem in the ATS source base
that it's not clear how mutual exclusion to non-stack data is guaranteed.
When you see a mutex in our code, it's generally not clear what all it's
protecting.

On Wed, Mar 20, 2019 at 4:17 PM Leif Hedstrom <zw...@apache.org> wrote:

> Don’t we have scoped mutexes already ?
>
> — Leif
>
> > On Mar 20, 2019, at 12:08, Walt Karas <wk...@verizonmedia.com.invalid>
> wrote:
> >
> > https://godbolt.org/z/6klEJn
> >
> > Useful for mutex-protected objects, where the mutex is only locked within
> > code blocks.  Could be used in this PR:
> >
> > https://github.com/apache/trafficserver/pull/5187
>
>

Re: Proposed addition to tsutil: Generic mutex-locking reference template

Posted by Walt Karas <wk...@verizonmedia.com.INVALID>.
I don't understand, by "standard mechanism" do you mean std::lock_guard?
LockRef uses that, see line 17 of the code.

https://godbolt.org/z/6klEJn

On Fri, Mar 22, 2019 at 1:10 PM Alan Carroll
<so...@verizonmedia.com.invalid> wrote:

> >
> > Yes, but I think that advantage of using a standard mechanism will be
> more
> > acceptable than a distinctly different mechanism. Better to take
> advantage
> > of existing habits.
> >
> >
>

Re: Proposed addition to tsutil: Generic mutex-locking reference template

Posted by Alan Carroll <so...@verizonmedia.com.INVALID>.
>
> Yes, but I think that advantage of using a standard mechanism will be more
> acceptable than a distinctly different mechanism. Better to take advantage
> of existing habits.
>
>

Re: Proposed addition to tsutil: Generic mutex-locking reference template

Posted by Walt Karas <wk...@verizonmedia.com.INVALID>.
I don't understand.  Part of the point is that you only have a reference to
the data (aPoint in this case) while it is also locked.

On Fri, Mar 22, 2019 at 10:16 AM Alan Carroll
<so...@verizonmedia.com.invalid> wrote:

> I'd be more tempted to specialize `std::lock_guard` for the class. E.g. so
> that
>
> std::lock_guard lock{aPoint};
>
> works an RAII lock on `aPoint`. I think that makes it easier because
> there's only one thing the user has to remember, "lock_guard".
>
> On Fri, Mar 22, 2019 at 9:19 AM Walt Karas <wkaras@verizonmedia.com
> .invalid>
> wrote:
>
> > scoped_lock is I think an alternative to lock_guard to avoid deadlock
> when
> > multiple mutexes must be locked at the same time.
> >
> > The point of the utility I propose is a fail-safe to make sure that
> > mutex-protected data is not accessed without locking the mutex, and
> clarity
> > about what data the mutex is protecting.
> >
> > On Thu, Mar 21, 2019 at 5:34 PM Jason Kenny <jk...@verizonmedia.com>
> > wrote:
> >
> > > I was thinking of https://en.cppreference.com/w/cpp/thread/scoped_lock
> > >
> > > Jason
> > >
> > > On Thu, Mar 21, 2019 at 4:53 PM Walt Karas <wkaras@verizonmedia.com
> > .invalid>
> > > wrote:
> > >
> > >> What's the name of it in the Standard Lib?
> > >>
> > >> On Thu, Mar 21, 2019 at 4:45 PM Jason Kenny <jkenny@verizonmedia.com
> > >> .invalid>
> > >> wrote:
> > >>
> > >> > I am unclear why it is needed. C++ 17 already has this in the std
> > >> library.
> > >> > these should be used instead of making our own
> > >> >
> > >> > Jason
> > >> >
> > >> > On Wed, Mar 20, 2019 at 4:17 PM Leif Hedstrom <zw...@apache.org>
> > wrote:
> > >> >
> > >> > > Don’t we have scoped mutexes already ?
> > >> > >
> > >> > > — Leif
> > >> > >
> > >> > > > On Mar 20, 2019, at 12:08, Walt Karas <wkaras@verizonmedia.com
> > >> > .invalid>
> > >> > > wrote:
> > >> > > >
> > >> > > > https://godbolt.org/z/6klEJn
> > >> > > >
> > >> > > > Useful for mutex-protected objects, where the mutex is only
> locked
> > >> > within
> > >> > > > code blocks.  Could be used in this PR:
> > >> > > >
> > >> > > > https://github.com/apache/trafficserver/pull/5187
> > >> > >
> > >> > >
> > >> >
> > >>
> > >
> >
>

Re: Proposed addition to tsutil: Generic mutex-locking reference template

Posted by Alan Carroll <so...@verizonmedia.com.INVALID>.
I'd be more tempted to specialize `std::lock_guard` for the class. E.g. so
that

std::lock_guard lock{aPoint};

works an RAII lock on `aPoint`. I think that makes it easier because
there's only one thing the user has to remember, "lock_guard".

On Fri, Mar 22, 2019 at 9:19 AM Walt Karas <wk...@verizonmedia.com.invalid>
wrote:

> scoped_lock is I think an alternative to lock_guard to avoid deadlock when
> multiple mutexes must be locked at the same time.
>
> The point of the utility I propose is a fail-safe to make sure that
> mutex-protected data is not accessed without locking the mutex, and clarity
> about what data the mutex is protecting.
>
> On Thu, Mar 21, 2019 at 5:34 PM Jason Kenny <jk...@verizonmedia.com>
> wrote:
>
> > I was thinking of https://en.cppreference.com/w/cpp/thread/scoped_lock
> >
> > Jason
> >
> > On Thu, Mar 21, 2019 at 4:53 PM Walt Karas <wkaras@verizonmedia.com
> .invalid>
> > wrote:
> >
> >> What's the name of it in the Standard Lib?
> >>
> >> On Thu, Mar 21, 2019 at 4:45 PM Jason Kenny <jkenny@verizonmedia.com
> >> .invalid>
> >> wrote:
> >>
> >> > I am unclear why it is needed. C++ 17 already has this in the std
> >> library.
> >> > these should be used instead of making our own
> >> >
> >> > Jason
> >> >
> >> > On Wed, Mar 20, 2019 at 4:17 PM Leif Hedstrom <zw...@apache.org>
> wrote:
> >> >
> >> > > Don’t we have scoped mutexes already ?
> >> > >
> >> > > — Leif
> >> > >
> >> > > > On Mar 20, 2019, at 12:08, Walt Karas <wkaras@verizonmedia.com
> >> > .invalid>
> >> > > wrote:
> >> > > >
> >> > > > https://godbolt.org/z/6klEJn
> >> > > >
> >> > > > Useful for mutex-protected objects, where the mutex is only locked
> >> > within
> >> > > > code blocks.  Could be used in this PR:
> >> > > >
> >> > > > https://github.com/apache/trafficserver/pull/5187
> >> > >
> >> > >
> >> >
> >>
> >
>

Re: Proposed addition to tsutil: Generic mutex-locking reference template

Posted by Walt Karas <wk...@verizonmedia.com.INVALID>.
scoped_lock is I think an alternative to lock_guard to avoid deadlock when
multiple mutexes must be locked at the same time.

The point of the utility I propose is a fail-safe to make sure that
mutex-protected data is not accessed without locking the mutex, and clarity
about what data the mutex is protecting.

On Thu, Mar 21, 2019 at 5:34 PM Jason Kenny <jk...@verizonmedia.com> wrote:

> I was thinking of https://en.cppreference.com/w/cpp/thread/scoped_lock
>
> Jason
>
> On Thu, Mar 21, 2019 at 4:53 PM Walt Karas <wk...@verizonmedia.com.invalid>
> wrote:
>
>> What's the name of it in the Standard Lib?
>>
>> On Thu, Mar 21, 2019 at 4:45 PM Jason Kenny <jkenny@verizonmedia.com
>> .invalid>
>> wrote:
>>
>> > I am unclear why it is needed. C++ 17 already has this in the std
>> library.
>> > these should be used instead of making our own
>> >
>> > Jason
>> >
>> > On Wed, Mar 20, 2019 at 4:17 PM Leif Hedstrom <zw...@apache.org> wrote:
>> >
>> > > Don’t we have scoped mutexes already ?
>> > >
>> > > — Leif
>> > >
>> > > > On Mar 20, 2019, at 12:08, Walt Karas <wkaras@verizonmedia.com
>> > .invalid>
>> > > wrote:
>> > > >
>> > > > https://godbolt.org/z/6klEJn
>> > > >
>> > > > Useful for mutex-protected objects, where the mutex is only locked
>> > within
>> > > > code blocks.  Could be used in this PR:
>> > > >
>> > > > https://github.com/apache/trafficserver/pull/5187
>> > >
>> > >
>> >
>>
>

Re: Proposed addition to tsutil: Generic mutex-locking reference template

Posted by Jason Kenny <jk...@verizonmedia.com.INVALID>.
I was thinking of https://en.cppreference.com/w/cpp/thread/scoped_lock

Jason

On Thu, Mar 21, 2019 at 4:53 PM Walt Karas <wk...@verizonmedia.com.invalid>
wrote:

> What's the name of it in the Standard Lib?
>
> On Thu, Mar 21, 2019 at 4:45 PM Jason Kenny <jkenny@verizonmedia.com
> .invalid>
> wrote:
>
> > I am unclear why it is needed. C++ 17 already has this in the std
> library.
> > these should be used instead of making our own
> >
> > Jason
> >
> > On Wed, Mar 20, 2019 at 4:17 PM Leif Hedstrom <zw...@apache.org> wrote:
> >
> > > Don’t we have scoped mutexes already ?
> > >
> > > — Leif
> > >
> > > > On Mar 20, 2019, at 12:08, Walt Karas <wkaras@verizonmedia.com
> > .invalid>
> > > wrote:
> > > >
> > > > https://godbolt.org/z/6klEJn
> > > >
> > > > Useful for mutex-protected objects, where the mutex is only locked
> > within
> > > > code blocks.  Could be used in this PR:
> > > >
> > > > https://github.com/apache/trafficserver/pull/5187
> > >
> > >
> >
>

Re: Proposed addition to tsutil: Generic mutex-locking reference template

Posted by Walt Karas <wk...@verizonmedia.com.INVALID>.
What's the name of it in the Standard Lib?

On Thu, Mar 21, 2019 at 4:45 PM Jason Kenny <jk...@verizonmedia.com.invalid>
wrote:

> I am unclear why it is needed. C++ 17 already has this in the std library.
> these should be used instead of making our own
>
> Jason
>
> On Wed, Mar 20, 2019 at 4:17 PM Leif Hedstrom <zw...@apache.org> wrote:
>
> > Don’t we have scoped mutexes already ?
> >
> > — Leif
> >
> > > On Mar 20, 2019, at 12:08, Walt Karas <wkaras@verizonmedia.com
> .invalid>
> > wrote:
> > >
> > > https://godbolt.org/z/6klEJn
> > >
> > > Useful for mutex-protected objects, where the mutex is only locked
> within
> > > code blocks.  Could be used in this PR:
> > >
> > > https://github.com/apache/trafficserver/pull/5187
> >
> >
>

Re: Proposed addition to tsutil: Generic mutex-locking reference template

Posted by Jason Kenny <jk...@verizonmedia.com.INVALID>.
I am unclear why it is needed. C++ 17 already has this in the std library.
these should be used instead of making our own

Jason

On Wed, Mar 20, 2019 at 4:17 PM Leif Hedstrom <zw...@apache.org> wrote:

> Don’t we have scoped mutexes already ?
>
> — Leif
>
> > On Mar 20, 2019, at 12:08, Walt Karas <wk...@verizonmedia.com.invalid>
> wrote:
> >
> > https://godbolt.org/z/6klEJn
> >
> > Useful for mutex-protected objects, where the mutex is only locked within
> > code blocks.  Could be used in this PR:
> >
> > https://github.com/apache/trafficserver/pull/5187
>
>

Re: Proposed addition to tsutil: Generic mutex-locking reference template

Posted by Leif Hedstrom <zw...@apache.org>.
Don’t we have scoped mutexes already ?

— Leif 

> On Mar 20, 2019, at 12:08, Walt Karas <wk...@verizonmedia.com.invalid> wrote:
> 
> https://godbolt.org/z/6klEJn
> 
> Useful for mutex-protected objects, where the mutex is only locked within
> code blocks.  Could be used in this PR:
> 
> https://github.com/apache/trafficserver/pull/5187