RAII object in constructor

M

Marcel Müller

I have a mutex lock that is implemented as RAII. I need to acquire the
mutex as long as the constructor runs. But for the /entire/ constructor
run not just the construction of the derived class.


Example:

#include <stdio.h>

// some mutex class
class Mutex
{
public:
// hold the Mutex (RAII)
struct Lock
{ Lock(Mutex& mtx)
{ puts("Lock()\n");
//...
}
~Lock()
{ puts("~Lock()\n");
//...
}
};
//...
};

// set of configuration parameters
struct Parameters
{ //...
};

class WorkerBase
{ //...
protected:
WorkerBase(const Parameters& params)
{ // do something with params...
}
};

class Worker : public WorkerBase
{
private:
static Parameters GlobalParams;
static Mutex ParamMtx; // protect the above

public:
// synchronized public access to the parameters
struct AccessParams : Mutex::Lock
{ AccessParams() : Mutex::Lock(ParamMtx) {}
operator Parameters&() { return GlobalParams; }
};

Worker() : WorkerBase(AccessParams())
{ AccessParams params; // !!! Mutex acquired twice!
// do something with params...
}
};

Parameters Worker::GlobalParams;
Mutex Worker::paramMtx;

int main()
{ Worker worker;
// do something with worker...
}


As expected the mutex is acquired twice. But this is bad, since the idea
is to sample a consistent set of parameters and not to use one parameter
set for the base class and some other one for the derived class.

Any ideas how to solve this?


Marcel
 
L

Luca Risolia

Marcel said:
As expected the mutex is acquired twice. But this is bad, since the idea
is to sample a consistent set of parameters and not to use one parameter
set for the base class and some other one for the derived class.

Any ideas how to solve this?

boost::recursive_mutex
 
M

Marcel Müller

boost::recursive_mutex

This won't help, since the mutex is released before the second attempt
to acquire it. This is a race condition.


Marcel
 
M

Marcel Müller

If you want to hold a single lock during the whole constructor, you can
just create it before calling the constructor. For example, make your
constructor private and provide a static Create function instead:

// static
Worker* Worker::Create() {
Mutex::Lock lock(ParamMtx);
return new Worker();
}

OK, a factory Function. This should do the Job. Thanks.

Using such Create() functions instead of calling constructors directly
has also some (arguably weak) benefits in shared libraries (increasing
binary compatibility).

Why should that increase compatibility?


Marcel
 
J

James Kanze

I have a mutex lock that is implemented as RAII. I need to acquire the
mutex as long as the constructor runs. But for the /entire/ constructor
run not just the construction of the derived class.

#include <stdio.h>
// some mutex class
class Mutex
{
public:
// hold the Mutex (RAII)
struct Lock
{ Lock(Mutex& mtx)
{ puts("Lock()\n");
//...
}
~Lock()
{ puts("~Lock()\n");
//...
}
};
//...
};

How is this fundamentally different from `std::mutex` and
`std::lock_guard`?
// set of configuration parameters
struct Parameters
{ //...
};
class WorkerBase
{ //...
protected:
WorkerBase(const Parameters& params)
{ // do something with params...
}
};
class Worker : public WorkerBase
{
private:
static Parameters GlobalParams;
static Mutex ParamMtx; // protect the above
public:
// synchronized public access to the parameters
struct AccessParams : Mutex::Lock
{ AccessParams() : Mutex::Lock(ParamMtx) {}
operator Parameters&() { return GlobalParams; }
};
Worker() : WorkerBase(AccessParams())
{ AccessParams params; // !!! Mutex acquired twice!
// do something with params...
}
};

The simplest solution (that I know) is to make the constructor
to Worker take an `AccessParams` argument:

Worker::Worker( AccessParams const& params = AccessParams() )
: WorkerBase( params )
{
}

There are a number of variants on this, depending on whether
`Worker` requires more arguments or not, but the basic idea is
for the most derived class' constructor to have the special
argument.

Note that if you ever use `Worker` as a temporary, the lock will
be held until the end of the full expression. This can be a bit
of a bother, if e.g. client code does something like:

func( Worker() );

The lock will be held during the entire call to func, which
could be a problem.
 
M

Marcel Müller

How is this fundamentally different from `std::mutex` and
`std::lock_guard`?

In the way that is supported by my platform. ;-)
C++11 is not an option here. However, writing a mutex class does not
take more than 10 minutes. But some other features of C++11 would be nice.

The simplest solution (that I know) is to make the constructor
to Worker take an `AccessParams` argument:

Worker::Worker( AccessParams const& params = AccessParams() )
: WorkerBase( params )
{
}

I have gone this way now, but the hack with the default argument is new
to me. OK, it requires the AccessParams interface to be const. As long
as I do not need the RAII object itself
Worker::Worker( Params& params = AccessParams() )
: WorkerBase( params )
{}
should do as well - assuming that AccessParams has an operator Params&().

Note that if you ever use `Worker` as a temporary, the lock will
be held until the end of the full expression. This can be a bit
of a bother, if e.g. client code does something like:

func( Worker() );

The lock will be held during the entire call to func, which
could be a problem.

True, thanks for the tip. I think I keep the way with the factory
function which is safe. It has the disadvantage that Worker can no
longer be allocated as local var on the stack, but this is not intended
in my case anyway.


Marcel
 
A

Alf P. Steinbach

I have a mutex lock that is implemented as RAII. I need to acquire the
mutex as long as the constructor runs. But for the /entire/ constructor
run not just the construction of the derived class.


Example:

#include <stdio.h>

// some mutex class
class Mutex
{
public:
// hold the Mutex (RAII)
struct Lock
{ Lock(Mutex& mtx)
{ puts("Lock()\n");
//...
}
~Lock()
{ puts("~Lock()\n");
//...
}
};
//...
};

// set of configuration parameters
struct Parameters
{ //...
};

class WorkerBase
{ //...
protected:
WorkerBase(const Parameters& params)
{ // do something with params...
}
};

class Worker : public WorkerBase
{
private:
static Parameters GlobalParams;
static Mutex ParamMtx; // protect the above

public:
// synchronized public access to the parameters
struct AccessParams : Mutex::Lock
{ AccessParams() : Mutex::Lock(ParamMtx) {}
operator Parameters&() { return GlobalParams; }
};

Worker() : WorkerBase(AccessParams())
{ AccessParams params; // !!! Mutex acquired twice!
// do something with params...
}
};

Parameters Worker::GlobalParams;
Mutex Worker::paramMtx;

int main()
{ Worker worker;
// do something with worker...
}


As expected the mutex is acquired twice. But this is bad, since the idea
is to sample a consistent set of parameters and not to use one parameter
set for the base class and some other one for the derived class.

A good approach, if practically possible, is to separate the global
parameters needed for instance construction, from the rest.

Then you can (hopefully) make those parameters read only, and avoid locking.

In passing, note that with C++ move semantics even a factory function
for "large" objects is not restricted to dynamic allocation, as long as
the produced type has accessible move constructor. And this in turn
means that it's possible to write a generic value factory function. C++
perfect argument forwarding comes in nicely for that, but is only needed
to make the usage transparent: it can also be done in C++03.


Cheers & hth.,

- Alf
 
L

Luca Risolia

This won't help

Sorry, I misunderstood your question. But this should work:

1. Add a Lock::Unlock() to make your lock like a std::unique_lock
2. Derive Worker from AccessibleParams
3. Use a function-try-block for the Worker constructor
4. Unlock() at the end of the Worker constructor's body

The solution should be exception-safe. See the comments for more details.

Here it is:

#include <stdio.h>

// some mutex class
class Mutex
{
public:
// hold the Mutex (RAII)
struct Lock // make it like std::unique_lock
{ Lock(Mutex& mtx)
{ puts("Lock()\n");
// owned = true;
//..
}
void Unlock(Mutex& mtx) // noexcept
{ puts("Unlock()\n");
// owned = false;
// ..
}
~Lock()
{ puts("~Lock()\n");
// if (!owned) return;
// ..
}
};
// ..
};

// set of configuration parameters
struct Parameters
{ //...
};

class WorkerBase
{ //...
protected:
WorkerBase(const Parameters& params)
{ // do something with params...
}
};

struct AccessParams_ : Mutex::Lock
{
AccessParams_();
operator Parameters&();
};

class Worker : private AccessParams_, public WorkerBase
{
private:
static Parameters GlobalParams;
static Mutex ParamMtx; // protect the above
friend AccessParams_;

public:
// synchronized public access to the parameters
// make AccessParams available as before
typedef AccessParams_ AccessParams;

// should an exception be thrown from WorkerBase or Worker constructors,
// the Lock will be released automatically via ~Lock()
Worker() try : WorkerBase(GlobalParams)
{
// do something with params...
Unlock(ParamMtx); // no exceptions: we can unlock explicitly
}
catch (...) { } // re-throws
};

Parameters Worker::GlobalParams;
Mutex Worker::paramMtx;

AccessParams_::AccessParams_() : Mutex::Lock(Worker::paramMtx) {}
AccessParams_::eek:perator Parameters&() {
return Worker::GlobalParams;
}

int main()
{ Worker worker;
// do something with worker...
}
 

Ask a Question

Want to reply to this thread or ask your own question?

You'll need to choose a username for the site, which only take a couple of moments. After that, you can post your question and our members will help you out.

Ask a Question

Members online

No members online now.

Forum statistics

Threads
473,769
Messages
2,569,580
Members
45,054
Latest member
TrimKetoBoost

Latest Threads

Top