Re: [Threads-devel] reverse_lock exception safety?

Subject: Re: [Threads-devel] reverse_lock exception safety?
From: Fredrik Orderud (forderud_at_[hidden])
Date: 2012-12-30 13:23:07

>
> You might very well be correct in that, reverse_lock::~reverse_lock in
> practice never throws when used in conjunction with the current mutex
> implementations provided with boost. But this is only part of the story.
> I'm pretty sure that there are boost users (besides me) that uses boost
> locks in conjunction with independent mutex implementations. And then your
> only guarantees are that the mutex adheres to the BasicLockable
> specification.
>
>    I agree with you that there is an potential issue.
>
>   My thought for safer design would be to move the locking code away from
> the destructor and to a separate close() method that must be called
> explicitly. This is a bit awkward, but results in a design where the
> destructor is guaranteed to never throw. The destructor should then check
> if close() has been called, and call terminate() if not. Note that this
> terminate() is semantically different from the current terminate(), since
> it's caused by a programmer error (forgot to close), compared to
> runtime-error (locking failure).
>
> What would be the expected behavior if close() is called and throws
> (locking error). E.g
>
> {
>   reverse_lock<mutex> rlk(mtx);
>   no_throw_fct();
>   rlk.close(); // throws
> }
>
> Should the rlk destructor call terminate()?
> If not, which recoverable action could take the user?
>

My suggestion would be that rlk.close() throws an exception that can be
handled by the client code like any other exception. The rlk destructor
should never throw under any cirumstances, but it should include an assert
to verify that close() has been explicitly called. A proposed patch is
attached (I've renamed the close() method into lock() to make the API more
intuitive).

I believe that C++11 std::thread follows a similar pattern in respect to
joining that must be done explicitly instead of being done automatically in
the destructor.

 This could be a patter to follow, but I would associate it to a different
> lock class, e.g. check_lock_guard. For reverse_lock I would prefer to just
> document that the Lockable parameter must not throw when lock() is called
> inside the destructor. What do you think?
>

I understand that there might be concerns regarding breaking existing code.
Could it be possible to somehow (static) assert against a "whitelist" of
mutexes that are guaranteed to not thow when used in a reverse_lock?

Regards,
Fredrik