Greatly simplify multithreaded access to global object

[Frederick Virchanza Gotham]

I don't know if I've re-invented the wheel here but I can't remember having seen something like this in the C++ standard library nor Boost nor wxWidgets.

Let's say we have a multi-threaded program, it has a main GUI thread and five worker threads, giving a total of six threads.

The program at all times has a status string, which is a global 'std::string' object. All six threads read and write the global status string.

I've tried to code a solution to greatly simplify allowing all six threads to read and write the global object. Let's start off with a basic function that gives access to the global string:

string &GetStatusString(void)
{
static string str;
return str;
}

So then from another function we can call 'GetStatusString' as follows:

void Func(void)
{
// This function accesses the global status string

auto &str = GetStatusString();

str += "monkey";
}

Of course this isn't thread-safe. I want to make this thread-safe, firstly by making the following two changes to 'Func' as follows:

void Func(void)
{
// This function accesses the global status string

auto str = GetStatusString(); // Change 1: no longer auto&

str.obj += "monkey"; // Change 2: 'str.obj' instead of 'str'
}

Next I change the function 'GetStatusString' as follows:

ObjectReserver<string> GetStatusString(void)
{
static string str;
static recursive_mutex mtx;

return ObjectReserver(mtx,str);
}

The last thing to do now is write the code for 'ObjectReserver'. Here's what I've got so far:

#include <mutex> // recursive_mutex, unique_lock

template<typename T>
class ObjectReserver final {

std::unique_lock<std::recursive_mutex> m_lock;

public:

T &obj;

ObjectReserver(std::recursive_mutex &argM, T &argO)
: m_lock(argM), obj(argO) {}

ObjectReserver(void) = delete;
ObjectReserver(ObjectReserver const &) = delete;
ObjectReserver(ObjectReserver &&) = delete;
ObjectReserver &operator=(ObjectReserver const &) = delete;
ObjectReserver &operator=(ObjectReserver &&) = delete;
ObjectReserver *operator&(void) = delete;
ObjectReserver const *operator&(void) const = delete;
};

I'm considering using this 'ObjectReserver' template class in a complex multithreaded program I'm currently writing. I thought I'd share this here though first just in case there's already something similar in Boost or whatever.

[Frederick Virchanza Gotham]

On Thursday, December 22, 2022 at 7:41:35 PM UTC, Frederick Virchanza Gotham wrote:
>
> template<typename T>
> class ObjectReserver final {
>
> std::unique_lock<std::recursive_mutex> m_lock;
>

I could have used "lock_guard" here, it's a more basic lock with all the functionality I need.

[Scott Lurndal]

Frederick Virchanza Gotham <cauldwel...@gmail.com> writes:
>
>I don't know if I've re-invented the wheel here but I can't remember having seen something like this in the C++ standard library nor Boost nor wxWidgets.
>
>Let's say we have a multi-threaded program, it has a main GUI thread and five worker threads, giving a total of six threads.
>
>The program at all times has a status string, which is a global 'std::string' object. All six threads read and write the global status string.
>

Rather than contending for single string updates, which can never scale,
keep a string private to each thread where each thread updates the
string as required without locking; when you need the full status
append the six strings into a single string. All you need to synchronize
is fetching the string from each thread and if you do it right, you'll
likely not even need to synchronize access from the main thread to
the strings (e.g. each thread has a pointer to the current status string
and when the worker thread updates the status, it uses an atomic exchange to replace
the pointer to the former status string with a pointer to the new
status - when the main thread fetches the status it will get either
the old or the new depending on how it races with the update). The
main thread can append the individual strings (or use/display each
status independently).

The recursive mutex solution seems fragile and not scalable to large
numbers of worker threads.

[Frederick Virchanza Gotham]

On Thursday, December 22, 2022 at 8:11:42 PM UTC, Scott Lurndal wrote:

> Rather than contending for single string updates, which can never scale,
> keep a string private to each thread where each thread updates the
> string as required without locking; when you need the full status
> append the six strings into a single string. All you need to synchronize
> is fetching the string from each thread and if you do it right, you'll
> likely not even need to synchronize access from the main thread to
> the strings (e.g. each thread has a pointer to the current status string
> and when the worker thread updates the status, it uses an atomic exchange to replace
> the pointer to the former status string with a pointer to the new
> status - when the main thread fetches the status it will get either
> the old or the new depending on how it races with the update). The
> main thread can append the individual strings (or use/display each
> status independently).
>
> The recursive mutex solution seems fragile and not scalable to large
> numbers of worker threads.

I wanted to keep my original post simple and so that's why I used 'std::string' as an example.

Really though I'm coding a man in the middle program that receives, modifies and forwards RS232 traffic. More than one thread has to access the various data structures that get created and edited in response to the receipt of RS232 traffic.

My program is already coded, and it actually works properly, but there are data races all over the camp. I coded it quickly to get it working to help me debug a microcontroller, but now I want to clean it up and so I think I'll use ObjectReserver for this.

[David Brown]

On 22/12/2022 20:41, Frederick Virchanza Gotham wrote:
>
> I don't know if I've re-invented the wheel here but I can't remember
> having seen something like this in the C++ standard library nor Boost
> nor wxWidgets.
>
> Let's say we have a multi-threaded program, it has a main GUI thread
> and five worker threads, giving a total of six threads.
>
> The program at all times has a status string, which is a global
> 'std::string' object. All six threads read and write the global
> status string.
>

I'd say that's your problem right there.

Only allow /one/ thing to change a given variable. Problem solved.
(Well, /almost/ solved - you might still need something to ensure
changes are seen atomically by readers, depending on how your strings
work and get updated.)


I try to think of programming like electronics. Devices have inputs and
outputs. It's okay to drive multiple inputs from one output, but if you
try to drive one line from multiple outputs, things are going to go
wrong (such as letting out the magic grey smoke that runs all electronics).

When you have six devices that all have an error indicator output, you
don't just join them together on one red LED. You make six error LEDs,
one for each device. Or you use a multiplexer, such as an OR gate, to
combine the signals safely.

Do the same in your programming.


So you have six separate status strings, and a "multiplexer" that
combines them in some way. Maybe it serialises them to a log file, or
joins them together for display, or prioritises them to show error
messages at higher priority than "okay" messages. Maybe this is done by
a single "update" function (which may need locking if called from
different threads), or it is in a separate thread connected by queues to
the original threads, or it is in a timer function called periodically
in the gui thread. There are many options.

But the worst idea is to have one thread set the status to "There's a
meltdown on it's way" only to have it immediately overwritten by another
thread saying "Everything's hunky-dory over here".

[Paavo Helde]

22.12.2022 21:41 Frederick Virchanza Gotham kirjutas:
>
> I don't know if I've re-invented the wheel here but I can't remember having seen something like this in the C++ standard library nor Boost nor wxWidgets.
>
> Let's say we have a multi-threaded program, it has a main GUI thread and five worker threads, giving a total of six threads.
>
> The program at all times has a status string, which is a global 'std::string' object. All six threads read and write the global status string.

[snipped implementation with a proxy containing a mutex lock]

I have done such things in the past, but in retrospect this was not the
best idea, mainly because it hides the thread-locking step and makes the
code harder to follow and verify for correctness.

Nowadays I would just make a dedicated member function of the
StatusManager class which would just append to the string under a mutex
lock. If it appears this is becoming a bottleneck, one can redesign the
member function to e.g. move the appended string pieces to some kind of
fast inter-thread queue. With a locked proxy like in your design it
would be harder to rewrite the functionality.

[Paavo Helde]

22.12.2022 22:41 David Brown kirjutas:

> But the worst idea is to have one thread set the status to "There's a
> meltdown on it's way" only to have it immediately overwritten by another
> thread saying "Everything's hunky-dory over here".

If you look at his code, he is appending to the status string, not
overwriting it. Of course, for a status string this is probably not the
best approach either, but I gather this was just meant as an
illustrative example.

[David Brown]

For my comments, it doesn't matter much how you change the common
variable - the fact that you are changing it from many places is the
core problem.

[Chris M. Thomasson]

On 12/22/2022 12:11 PM, Scott Lurndal wrote:
> Frederick Virchanza Gotham <cauldwel...@gmail.com> writes:
>>
>> I don't know if I've re-invented the wheel here but I can't remember having seen something like this in the C++ standard library nor Boost nor wxWidgets.
>>
>> Let's say we have a multi-threaded program, it has a main GUI thread and five worker threads, giving a total of six threads.
>>
>> The program at all times has a status string, which is a global 'std::string' object. All six threads read and write the global status string.
>>
>
> Rather than contending for single string updates, which can never scale,
> keep a string private to each thread where each thread updates the
> string as required without locking; when you need the full status
> append the six strings into a single string.

Yes. This is analogous to split counters. Each thread keeps its own
count. If you want to know the sum, well, sum up all of the threads
counters. It scales fairly decently. A lot better than a single shared
counter! :^)

[Chris M. Thomasson]

On 12/22/2022 12:11 PM, Scott Lurndal wrote:

recursive mutex makes be get a bit sick from time to time. Brings back
some memories about having to debug some deeply nested locking issues.
God damn it was bad.

[Frederick Virchanza Gotham]

On Thursday, December 22, 2022, I wrote:
> I don't know if I've re-invented the wheel here
> but I can't remember having seen something
> like this in the C++ standard library nor Boost
> nor wxWidgets.

I've written code that will work on C++11, and which is optimised for C++17 which has guaranteed return value optimisation:

#ifndef HEADER_INCLUSION_GUARD_RESERVER
#define HEADER_INCLUSION_GUARD_RESERVER

#include <mutex> // recursive_mutex, lock_guard, unique_lock
#include <utility> // move

template<typename T>
class Reserver final {

Reserver(void) = delete;
Reserver(Reserver const &) = delete;
//Reserver(Reserver &&) = delete; - see below
Reserver &operator=(Reserver const &) = delete;
Reserver &operator=(Reserver &&) = delete;
Reserver const volatile *operator&(void) const volatile = delete;

T &obj;

#ifdef __cpp_guaranteed_copy_elision
std::lock_guard<std::recursive_mutex> m_lock;
Reserver(Reserver &&arg) = delete;
#else

std::unique_lock<std::recursive_mutex> m_lock;
public:

Reserver(Reserver &&arg)
: m_lock( std::move(arg.m_lock) ),
obj(arg.obj) {}
#endif

public:

Reserver(std::recursive_mutex &argM, T &argO)
: m_lock(argM), obj(argO) {}

};

#endif // HEADER_INCLUSION_GUARD_RESERVER

Reserver<int> Func(void)
{
static std::recursive_mutex mtx;
static int i = 7;

return Reserver<int>(mtx,i);
}

int main(void)
{
auto i = Func();
}

[Mut...@dastardlyhq.com]

On Sat, 31 Dec 2022 07:41:30 -0800 (PST)
Frederick Virchanza Gotham <cauldwel...@gmail.com> wrote:
>On Thursday, December 22, 2022, I wrote:
>> I don't know if I've re-invented the wheel here
>> but I can't remember having seen something
>> like this in the C++ standard library nor Boost
>> nor wxWidgets.
>
>I've written code that will work on C++11, and which is optimised for C++17
>which has guaranteed return value optimisation:

Thats nice. What does "return value optimisation" mean in practice then?

[David Brown]

See if this helps: <https://en.cppreference.com/w/cpp/language/copy_elision>