CODEWITHSUNDEEP
c++multithreadingconstructorthisself-reference
q126y
q126y

Reputation: 1671

Referencing this pointer in constructor

Following is another example of forward declaration, which might be useful if the application needs a self-sustaining array of objects which is able to add and remove objects from itself during run-time:

File a.h:

class A {
public:
    static A *first, *last;
    A *previous, *next;

    A();
    ~A();
};

File a.cpp:

#include "a.h"

A *A::first=0, *A::last=0; // don't put the word static here, that will cause an error

A::A() {
    if(first==0) first=this; //first A created
    previous=last;
    if(previous != 0) previous->next=this;
    last=this;
    next=0;
}

A::~A() {
    if(previous != 0) previous->next=next;
    if(next != 0) next->previous=previous;
}

From : https://en.wikipedia.org/wiki/Circular_dependency#Self-reference_example

I think implementation of A, has an issue. While first instance of A is being constructed, if some other thread references A::first, it will lead to unexpected behaviour. Please correct if I am wrong.

Also, how can this issue be resolved? Thanks.

Upvotes: 4

Views: 131

Answers (4)

SergeyA
SergeyA

Reputation: 62553

The problem with the answers here is that they don't point to the fact that previous and next are public members - which means, no protection by mutextes within constructor and destructor can make the class fully thread safe. The only way to make this class thread-safe is to hide those members and provide methods to access to them - each having a synchronization primitive inside.

Upvotes: 1

Serge Ballesta
Serge Ballesta

Reputation: 148870

This code is definitely non thread-safe as others stated in comments. Be it first object or not, it 2 threads try to create or delete a A object at the same time, you get undefined behaviour, because two different threads use and change same static value without any synchronization.

What could be done? As always to two same options:

  • document the class (at least contructor and destructor) as not being thread safe. Since them it is the caller's responsability to ensure that only one thread can access A objects at the same time. Say differently, it means that A will be safe only in a single treaded program.
  • add synchronization inside the class itself to make it thread safe. As creation and destruction manipulate static members, you need a global mutex for all objects of the class, say differently a class static one.

And as @zvone noticed in comment, the destructor could make first and last become dangling pointers when you delete first or last element of the chain.

Destructor (non thread safe version) should be:

A::~A() {
    if(previous != 0) previous->next=next;
    if(next != 0) next->previous=previous;
    if (first == this) first = next;
    if (last == this) last = previous;
}

A thread-safe version could be:

File a.h

class A {
public:
    static A *first, *last;
    A *previous, *next;
    static std::mutex mut;

    A();
    ~A();
};

File a.cpp:

#include "a.h"

A *A::first=0, *A::last=0; // don't put the word static here, that will cause an error
std::mutex A::mut;

A::A() {
    mut.lock()
    if(first==0) first=this; //first A created
    previous=last;
    if(previous != 0) previous->next=this;
    last=this;
    next=0;
    mut.unlock();
}

A::~A() {
    mut.lock()
    if(previous != 0) previous->next=next;
    if(next != 0) next->previous=previous;
    if (first == this) first = next;
    if (last == this) last = previous;
    mut.unlock();
}

Upvotes: 2

Thomas Sparber
Thomas Sparber

Reputation: 2917

Yes that's right. They Need to be protedted. Additionally, previous and next Need to be protected too. e.g.

A::~A() {
    unique_lock<std::mutex> locked(A::A_mutex);
    if(previous != 0) previous->next=next;
    if(next != 0) next->previous=previous;
}

where A_mutex is a static member.

Upvotes: 0

Roman Zaitsev
Roman Zaitsev

Reputation: 1448

You need to declare first and last variables as atomic using for example std::atomic
Or protect them with mutex

Upvotes: 0

Related Questions