Extending the generic thread-safe mutexed_obj class

Except for the volatile "trick", I’ve used multiple times all approaches I’ve documented in my thoughts about sharing data between threads.

The one I’m most comfortable with, is the mutexed_obj, as it makes much easier to document which data can be accessed safely between threads, and couples together all data that are owned by the same mutex.

After using mutexed_obj in different places, I’ve noticed that there where some use-cases it could not support and design compromises I was not happy about, especially when I wanted to integrate it in already existing code.

#include <mutex>

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 <typename... Objs, class F>
		friend auto lock(F f, Objs&&... objs);
};

template <typename... Objs, class F>
auto lock(F f, Objs&&... objs){
	std::scoped_lock lck(objs.m...);
	return f(objs.obj...);
}

int main(){
	mutexed_obj<int, std::mutex> mo1(1);
	mutexed_obj<int, std::recursive_mutex> mo2(2);

	auto v1 = lock([](int&, int&){ return 1;}, mo2, mo1);
	auto v2 = lock([](const int&, int&){return 1;}, mo2, mo1);
	auto v3 = lock([](const int&){return 1;}, mo2);
}

The mutex and scoped_lock are both hidden, there is no way to mess with them, but there are situations where one wants to interact with them.

For example, one might want to try_lock, or to wait on a condition variable. In both cases, a lock_guard is not the correct type of lock, and the user needs to interact with it.

Adding a try_lock function

Another missing functionality of mutexed_obj is the possibility to try to lock the mutex and execute some actions. While some claim "do; or do not; there is no try", there exists such code. There also exist such functionality in the standard library, and not supporting such use-case limits the ability to encapsulate uniformly the data within mutexed_obj.

I had to do some experiments with functions signatures, as there are multiple possible approaches.

Returning a boolean value

The easiest way to inform the user if the mutex has been loocked, and f called, it’s to return the value of std::unique_lock::owns_lock():

template<class T, class mutex = std::mutex>
class mutexed_obj {
	// as before
	public:
		template<class F>
		friend bool try_lock(mutexed_obj<T, mutex>& mo, F f){
			std::unique_lock lck(mo.m, std::try_to_lock);
			if(lck.owns_lock()){
				f(mo.obj, lck);
				return true;
			}
			return false;
		}
		// overload for const mutexed_obj<T, mutex>&
};

The big advantage of this function signature is that it makes easy to write code like if(!try_lock(v, f)) { std::cerr << "failed\n";} or even while(!try_lock(v, f)) { /* do something else */}"

Unfortunately, the signature is inconsistent with the other lock functions.

In the degenerate case that f returns bool, one could write `bool b = try_lock(mo, f); and not realize that the stored value is not the computed result.

Also, the fact that the return value of f is discarded/cannot be used, is a problem. This is especially true for types with complex or costly constructors, as for those an in-out parameter/two-step approach for initializing is not always as efficient, or even possible.

Returning std::optional or std::variant

template<class T, class mutex = std::mutex>
class mutexed_obj {
	// as before
	public:
		template<class F, class G>
		friend auto try_lock(mutexed_obj<T, mutex>& mo, F f){
			std::unique_lock l(mo.m, std::try_to_lock);
			return std::optional(f(mo.obj, l)) : std::nullopt;
		}
		// overload for const mutexed_obj<T, mutex>&
};

Contrary to returning boolean values, with this approaches we can use the return value of f as intended. Unfortunately, those function signatures are not compatible with the void return type, and if f returns a reference to something.

Also, there are use-cases where using std::optional or std::variant is not necessary, as there might already a "default" value the user might want to use for its own logic, like a nullptr or an object with a specific value.

As both std::variant and std::optional are available only from C++17, it is more difficult to use such function signature in more constrained environments where maybe only C++11 is available (unfortunately those exists too, even if it is not an hard requirement for mutexed_obj), even if it is possible to backport those types to some extent.

Take two callbacks

This is the approach that probably makes more sense: take a second callback function:

template<class T, class mutex = std::mutex>
class mutexed_obj {
	// as before
	public:
		template<class F, class G>
		friend auto try_lock(mutexed_obj<T, mutex>& mo, F f, G g){
			std::unique_lock l(mo.m, std::try_to_lock);
			return l.owns_lock() ? f(mo.obj, l) : g();
		}
		// overload for const mutexed_obj<T, mutex>&
};

The advantage of this approach is that all responsibilities are given to the caller. The drawback is, that defining a function g() is not always obvious, as it needs to have the same return type of f, or at least have a common type with the return type of f.

It means, for example, that f cannot return some data, and g() be a void function.

Also, the signature of try_lock is more complex, as there is an additional template parameter.

The advantage is that both of the previous approaches can be implemented with this implementation. This does not demonstrate anything, but it is a strong hint that this is the correct building blocks we were searching because it minimizes the number of functions in our interface.

Returning a boolean value:

bool try_lock2(mutexed_obj<T, mutex>& mo, F f){
	return try_lock(mo,
		[](T&& t){f(t);return true;},
		[](){return false;}
	);
}
// overload for const mutexed_obj<T, mutex>&

Returning std::optional (or std::variant).

auto try_lock3(mutexed_obj<T, mutex>& mo, F f){
	return try_lock(mo,
		[](T&& t){return std::optional(f(t));},
		[](){return std::nullopt;}
	);
}
// overload for const mutexed_obj<T, mutex>&

or

auto try_lock4(mutexed_obj<T, mutex>& mo, F f){
	return try_lock(mo,
		f,
		[](){return std::nullopt;}
	);
}
// overload for const mutexed_obj<T, mutex>&

which flattens std::optional<std::optional<>> to std::optional<> in case f already returns std::optional of some type, which makes more sense in some use-cases.

Order of arguments

I strongly believe that writing code like

if(doit){
	// do thist
	// do that
	// and that
	// call this
	// call that
	// and also this
	// and maybe something else
	// etc
	// etc
	// etc
	return true;
} else {
	return false; // only one or two short statements
}

is much harder to read than

if(!doit){
	return false; // only one or two short statements
} else {
	// do thist
	// do that
	// and that
	// call this
	// call that
	// and also this
	// and maybe something else
	// etc
	// etc
	// etc
	return true;
}

because, especially if there are also nested conditions, the probability to remember immediately what is attached at the else is inversely proportional to the number of lines. If the else and if statement cannot be read on a single screen, then it’s almost a certainty.

While one might argue that if and else blocks should not be that big, they exist.

This approach holds in other places too. I find it generally way more legible having short statements at the beginning, and the longer at the end, instead of the contrary. I find harder code especially illegible if those are mixed, as it is much easier to overlook small statements in the middle.

I believe the main reason is not only the physical distance in line with numbers but also because generally more lines mean there is more going on (or at least more information one has to process to understand what is happening). While reading and deciphering all those lines and statements, as generally, people can hold only a limited amount of state in mind, we tend to forget to what conditions (the first conditions) those are attached, thus when reading an else we need to check back where we were, and interrupt the flow.

Long story short, I do not like the variadic signature of lock, because generally function names (or lambdas) are longer than variable names.

I would like much more to write

lock(mo1, mo2, [](int i, int j){
	return i;
});

or

lock(
	mo1, mo2,
	[](int i, int j){
		return i;
	}
);

instead of

lock([](int i){
	return i;
}, mo1, mo2);

or

lock(
	[](int i){
		return i;
	},
	mo1, mo2
);

Adding lines does not improve the readability.

Unfortunately, because those are the rules of the C++ language, variadic packs must be the last argument.

The only possible workaround is to save those parameters somewhere, like std::tuple, and pass it as the first parameter.

It is certainly doable, and hopefully, the compiler can elide all indirections, but it makes writing code harder.

Instead of

lock([](int i, int j){
	return i;
}, mo1, mo2);

we have would have

lock(std::forward_as_tuple(mo1, mo2), [](int i, int j){
	return i;
});

As I would use std::tuple as main parameter type also for the non-variadic functions (I like consistencies), writing std::forward_as_tuple is distraction. Also by looking at the signature of the function, the first guess would be to use std::make_tuple.

The solution would be to alias std::forward_as_tuple that hides the fact we are creating a tuple (as the function should be used only together with the lock-functions) and with a shorter name, as ideally, we would write nothing.

As aliasing a function is not really possible in C++, manually wrapping std::forward_as_tuple for mutexed_obj seems to be the easiest approach.

Note: Yes, function pointers are a thing, but globals are not optimized away and pose a security risk, Microsoft, for example, recommends using EncodePointer and DecodePointer.

template <class... Types>
constexpr auto collect(Types&&... args) {
	// maybe static_assert that args are all of type mutexed_obj
	return std::forward_as_tuple(args...);
}

and the code can be written as

lock(collect(mo1, mo2), [](int i, int j){
	return i;
});

Unfortunately, the implementation of the variadic lock is also more complex, but at least it’s hidden from the end user.

The whole class

#include <mutex>
#include <tuple>

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 <typename... I, class F>
		friend auto lock(std::tuple<I...> t, F f);

		// try_lock
		template<class F, class G, class lock_policy>
		friend auto try_lock(mutexed_obj<T, mutex>& mo, lock_policy p, F f, G g) {
			static_assert(!std::is_same<lock_policy, std::defer_lock_t>::value, "std::defer_lock not permitted");
			static_assert(!std::is_same<lock_policy, std::adopt_lock_t>::value, "std::adopt_lock_t not permitted");
			std::unique_lock l(mo.m, p);
			return l.owns_lock() ? f(mo.obj) : g();
		}
		template<class F, class G, class lock_policy>
		friend auto try_lock(const mutexed_obj<T, mutex>& mo, lock_policy p, F f, G g) {
			static_assert(!std::is_same<lock_policy, std::defer_lock_t>::value, "std::defer_lock not permitted");
			static_assert(!std::is_same<lock_policy, std::adopt_lock_t>::value, "std::adopt_lock_t not permitted");
			std::unique_lock l(mo.m, p);
			return l.owns_lock() ? f(mo.obj) : g();
		}
};
template <class... Types>
constexpr auto collect(Types&&... args) {
	return std::forward_as_tuple(args...);
}
template <typename... I, class F>
auto lock(std::tuple<I...> t, F f) {
	auto lck = std::apply([&](auto&&... objs){ return std::scoped_lock(objs.m...);}, t);
	return std::apply([&](auto&&... objs){ return f(objs.obj...);}, t);
}
template<class T, class mutex, class F>
auto lock(mutexed_obj<T, mutex>& mo, F f) {
	return lock(collect(mo), f);
}
template<class T, class mutex, class F>
auto lock(const mutexed_obj<T, mutex>& mo, F f) {
	return lock(collect(mo), f);
}

template<class T, class mutex, class F, class G>
auto try_lock(mutexed_obj<T, mutex>& mo, F f, G g) {
	return try_lock(mo, std::try_to_lock, f, g);
}
template<class T, class mutex, class F, class G>
auto try_lock(const mutexed_obj<T, mutex>& mo, F f, G g) {
	return try_lock(mo, std::try_to_lock, f, g);
}

An example on how it can be used:

int fun(int i){
	return i;
}
int gfun(int i, int j){
	return i;
}

int main(){
	mutexed_obj<int> m1(42);
	const mutexed_obj<int> m2(42);
	mutexed_obj<int, std::timed_mutex> m3(42);

	// lock
	lock(m1, [](int& i){return i;});
	lock(m2, [](const int& i){return i;});

	lock(m1, [](int i){return i;});
	lock(m2, [](int i){return i;});

	lock(collect(m1, m2), gfun);
	lock(collect(m2, m1), gfun);

	// try_lock

	auto create_default = [](){return 42;};

	try_lock(m1, fun, create_default);
	try_lock(m2, fun, create_default);

	try_lock(m1, std::try_to_lock, fun, create_default);
	try_lock(m2, std::try_to_lock, fun, create_default);

	auto timeout = std::chrono::seconds(2);
	try_lock(m3, timeout, fun, create_default);
}

Notice that the class does not satisfy any possible need.

The standard library provides other threading primitives that plays together with std::mutex and locks. I do not think that it is possible to tie all those types together and avoid most possible misuses, ie. detect as many errors as possible at compile-time, and still satisfy every use-case.

Thus this class should not try to encapsulate every functionality, even if it is easy to "just add another function"…​