0

If I have a private member of a class which may be modified by a background thread, and a getter for said private member, can I use a const reference to that private member for the getter or must I protect the getter with a mutex to ensure safety? Here is some example code. Note that I am not using C++11 so I do not have access to those features. I am aware of std::atomic and std::lock_guard and their uses but the code I am working on at this moment uses C++03.

It is worth noting that shared_member's type int is more of a placeholder for simplicity

If there is a nicer way to ensure safety than the get_copyof_shared_int() method, I am all ears. However if the reference will be safe that will also work, I only want to ensure safety.

#include <pthread.h>

using namespace std;

class testclass{

public:

// return reference to shared_member (provided only as example of ideal getter)

    inline const int& get_shared_member () const{
        return shared_member;
    }

// return copy of shared_member (example of getter known to be thread safe )

    inline const int get_copyof_shared_int () {

        pthread_mutex_lock(&shared_int_mutex);

        int shared_member_copy = shared_member;

        pthread_mutex_unlock(&shared_int_mutex);

        return shared_member_copy;

    }

// initializes shared_member and mutex, starts running background_thread
    void init(int);

private:

    volatile int shared_member; //should be marked volatile because it is used in background_thread()

    pthread_mutex_t shared_int_mutex;

    // thread which may modify shared_member 

    static void *background_thread(void *arg);

};
wolf
  • 127
  • 7
  • You'll need to use some synchronization mechanism (e.g. mutex) to prevent race conditions on accessing the data. – πάντα ῥεῖ Dec 26 '13 at 19:16
  • 1
    Any time you have a variable that may be read and written in separate threads of execution, you should provide synchronization/locking protection, regardless of whether the variable in question is a global, a class member (static or not), a stack variable, dynamically allocated or whatever... – twalberg Dec 26 '13 at 19:38

2 Answers2

1

Taking a reference to it is essentially like passing a pointer (references are usually implemented as a contractual layer over pointers).

That means there is no guarantee that your thread won't read the value at some inconvenient time such as when the other thread, on a different core, is in the middle of writing to it.

Also, you might want to look into C++1's and headers.

I would also advise against inlining a getter with a mutex.

const int get_copyof_shared_int () {

    std::lock_guard<std::mutex> lock(shared_int_mutex);

    return shared_int;

}
kfsone
  • 23,617
  • 2
  • 42
  • 74
  • +1 for the C++11 features. `std::atomic` could be used for this situation. Yes, I revised my question because the code I'm working on is C++03 so no std::lock_guard or std::atomic. but those are both excellent for those that have the option to use C++11! Also, I appreciate the criticism about inlining the getter with a mutex as I don't 100% understand when/when not to inline, do you have a more detailed answer as to why it is not exactly best practice to inline my second (thread-safe) getter? – wolf Dec 26 '13 at 23:05
  • Inlining: For the most part, don't: let the compiler decide. You only need to start optimizing if you have done profiling that tells you that the compiler is choosing poorly or if you are creating a complex piece of code that you're really certain you want the compiler to always emit inline rather than use a function for. – kfsone Dec 27 '13 at 09:29
  • In the case of the mutex: You're creating a resource bottleneck and presumably a potential conditional branch. If you absolutely know that it's only going to be called in one place, then the inline hint may be fine, but given that it's not private it looks like something you'll be calling from various sites around your code. If you've done profiling and found a vast disparity between the contention frequency of some call paths to the getter vs others, then inlining would be advisable. Otherwise, you're littering your code (and branch cache) with unique manipulations of the mutex. – kfsone Dec 27 '13 at 09:36
1

Unfortunately yes, technically you should protect the getter since int operations are not guaranteed to be atomic.

As for the getter, it looks about as simple as it gets, although I'm not sure why you have two different getters.

EDIT: Don't forget to mark your shared variable as volatile as pointed out in the link above. Otherwise the optimizing compiler may make some improper optimizations since it can assume (wrongly in your case) the variable won't be set by another thread.

Community
  • 1
  • 1
TypeIA
  • 16,916
  • 1
  • 38
  • 52
  • As I clarified in my edit, the first getter would be a more ideal getter I would use if shared_member was not accessed by other threads. Thanks for the tip about the volatile keyword, and for the link to the other question! That really gets down to the bottom of it. – wolf Dec 26 '13 at 23:09