Skip to content

Commit

Permalink
c++11: don't throw from the reverselock destructor
Browse files Browse the repository at this point in the history
noexcept is default for destructors as of c++11. By throwing in reverselock's
destructor if it's lock has been tampered with, the likely result is
std::terminate being called. Indeed that happened before this change.

Once reverselock has taken another lock (its ctor didn't throw), it makes no
sense to try to grab or lock the parent lock. That is be broken/undefined
behavior depending on the parent lock's implementation, but it shouldn't cause
the reverselock to fail to re-lock when destroyed.

To avoid those problems, simply swap the parent lock's contents with a dummy
for the duration of the lock. That will ensure that any undefined behavior is
caught at the call-site rather than the reverse lock's destruction.

Barring a failed mutex unlock which would be indicative of a larger problem,
the destructor should now never throw.
  • Loading branch information
theuni committed Jan 5, 2016
1 parent 76ac35f commit 89f71c6
Show file tree
Hide file tree
Showing 2 changed files with 10 additions and 11 deletions.
5 changes: 4 additions & 1 deletion src/reverselock.h
Original file line number Diff line number Diff line change
Expand Up @@ -15,17 +15,20 @@ class reverse_lock

explicit reverse_lock(Lock& lock) : lock(lock) {
lock.unlock();
lock.swap(templock);
}

~reverse_lock() {
lock.lock();
templock.lock();
templock.swap(lock);
}

private:
reverse_lock(reverse_lock const&);
reverse_lock& operator=(reverse_lock const&);

Lock& lock;
Lock templock;
};

#endif // BITCOIN_REVERSELOCK_H
16 changes: 6 additions & 10 deletions src/test/reverselock_tests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -42,22 +42,18 @@ BOOST_AUTO_TEST_CASE(reverselock_errors)
BOOST_CHECK(failed);
BOOST_CHECK(!lock.owns_lock());

// Make sure trying to lock a lock after it has been reverse locked fails
failed = false;
bool locked = false;
// Locking the original lock after it has been taken by a reverse lock
// makes no sense. Ensure that the original lock no longer owns the lock
// after giving it to a reverse one.

lock.lock();
BOOST_CHECK(lock.owns_lock());

try {
{
reverse_lock<boost::unique_lock<boost::mutex> > rlock(lock);
lock.lock();
locked = true;
} catch(...) {
failed = true;
BOOST_CHECK(!lock.owns_lock());
}

BOOST_CHECK(locked && failed);
BOOST_CHECK(failed);
BOOST_CHECK(lock.owns_lock());
}

Expand Down

0 comments on commit 89f71c6

Please sign in to comment.