MSVC 2003: ctor-try-block error: possible compiler bug
Kirill Sukhonosenko
I face a problem in my production code. I could deduce this problem to
program shown below. It seems, that try-block in constructor doesnt
work as it should (compared to case where no try block exists at all).
I tested this small program on my MSVC .NET Pro 2003 (and separately on
PC with MSVC2003 SP1 installed). In both cases I experienced the same
behaviour - access violation. Please see comment inside the program
body. To get this sample work - just comment out try block in ctor.
Have anyone faced such behaviour before? Is there known workarounds?
Thanks.
----
// ref_count.cpp : ctor try block invalid behaviour test
// if ctor uses try block (X::X), program crashes, trying to
dereferencing danglig pointer
// If try block is disabled, everything goes fine.
// I experienced this problem in my production code which used
boost::intrusive_ptr class.
// I wrote similar class (CountingPtr) for simplicty's sake (and to
kill dependencies) and results are the same - program crashes
// tested on compilers: MS VS.NET 2003 (both original & SP1)
#include <iostream>
#include <cassert>
using namespace std;
struct IRefCounted
{
virtual void addRef() = 0;
virtual void release() = 0;
virtual ~IRefCounted() {};
};
struct ILogger : public IRefCounted
{
void virtual write(const char * text) = 0;
};
class Logger : public ILogger
{
int m_ref;
~Logger(){}
public:
Logger() : m_ref(0)
{
}
void addRef()
{
m_ref++;
}
void release()
{
if(--m_ref == 0)
delete this;
}
void write(const char * text)
{
cout << text << endl;
}
};
template<typename T>
class CountingPtr
{
T * m_t;
public:
CountingPtr(T * t, bool addRef = true) : m_t(t)
{
assert(t!=0);
if(addRef)
t->addRef();
}
~CountingPtr()
{
if(m_t)
m_t->release();
}
CountingPtr&operator=(const CountingPtr & rhs)
{
if(this == &rhs)
assert(false);
else
{
m_t = rhs.m_t;
m_t->addRef();
}
return *this;
}
T * operator->()
{
return m_t;
}
CountingPtr(const CountingPtr & rhs)
{
*this = rhs;
}
};
typedef CountingPtr<ILogger> CLoggerPtr;
void use(CLoggerPtr logger)
{
logger->write("use");
}
class Loggable
{
public:
Loggable(CLoggerPtr logger) : m_logger(logger)
{}
private:
CLoggerPtr m_logger;
};
class X : public Loggable
{
public:
X(CLoggerPtr logger)
try : Loggable(logger) // comment try
block out to
get working code!
{}
catch(std::runtime_error & )
{
throw;
};
};
int main(int argc, char* argv[])
{
try
{
CLoggerPtr logger(new Logger());
use(logger);
{
X y1(logger); // THIS line decrements ref counter by
2 in case try
block is active
}
{
X y2(logger); // OOPS IS HERE! If try block is in
act, program tries
to reference dangling pointer
}
return 0;
}
catch(exception & ex)
{
cout << "Exception: " << ex.what() << endl;
return EXIT_FAILURE;
}
catch(...)
{
cout << "(...) exception" << endl;
return EXIT_FAILURE;
}
}
--
[ See http://www.gotw.ca/resources/clcm.htm for info about ]
[ comp.lang.c++.moderated. First time posters: Do this! ]
Ulrich Eckhardt
> struct IRefCounted
> {
> virtual void addRef() = 0;
> virtual void release() = 0;
>
> virtual ~IRefCounted() {};
> };
>
> struct ILogger : public IRefCounted
> {
> void virtual write(const char * text) = 0;
> };
Why force implementors of ILogger to also implement IRefCounted? C++ has
multiple inheritance! In fact, I'd rather have made IRefCounted a concrete
class with a concrete implementation that invokes "delete this;" - a
virtual dtor will do the right thing!
> template<typename T>
> class CountingPtr
> {
> T * m_t;
>
> public:
> CountingPtr(T * t, bool addRef = true) : m_t(t)
> {
> assert(t!=0);
> if(addRef)
> t->addRef();
> }
>
> ~CountingPtr()
> {
> if(m_t)
> m_t->release();
> }
Hmmm, weird. In the ctor, you assert() that m_t is not null and here you
check it again?
> CountingPtr&operator=(const CountingPtr & rhs)
> {
> if(this == &rhs)
> assert(false);
> else
> {
> m_t = rhs.m_t;
> m_t->addRef();
> }
> return *this;
> }
This is very nontypical. Firstly, self-assignment, even though it rarely has
a proper use, can much easier be handled gracefully without asserting:
if(this!=&rhs)
{ ...do assignment... }
return *this;
Else, you simply overwrite the current value of m_t without first
deregistering from it (using release()). This is almost certainly wrong!
> CountingPtr(const CountingPtr & rhs)
> {
> *this = rhs;
> }
Note that here, you will also have to init m_t to zero before doing the
assignment!
Lastly, I'm missing a default ctor. The one generated by the compiler will
surely not do what you want, in particular not init m_t to zero.
I'm sorry if this doesn't directly address your problem, but browsing the
code those were some obvious mistakes I found and that I'd fix first. I
guess the rest will only turn out to be results of that.
Uli
Kirill Sukhonosenko
Sure, above mentioned comments are right. I can be excused only by my
wish to write quick & dirty code for my own case but not for everyday
use. I think, implementation shown below will satisfy further reviewers
from viewpoint of CountingPtr correctness. I believe it is the only
issue relevant to stated problem. Hope I have fixed all issues. Please
see below corrected version:
-----
// ref_count.cpp : ctor try block invalid behaviour test
// if ctor uses try block (X::X), program crashes, trying to
dereferencing danglig pointer
// If try block is disabled, everything goes fine.
// I experienced this problem in my production code which used
boost::intrusive_ptr class.
// I wrote similar class (CountingPtr) for simplicty's sake (and to
kill dependencies) and results are the same - program crashes
// tested on compilers: MS VS.NET 2003 (both original & SP1)
// Sukhonosenko Kirill (kirill.su...@a4vision.com)
#include <iostream>
#include <cassert>
using namespace std;
struct IRefCounted
{
virtual void addRef() = 0;
virtual void release() = 0;
virtual ~IRefCounted() {};
};
struct ILogger : public IRefCounted
{
void virtual write(const char * text) = 0;
};
class Logger : public ILogger
{
int m_ref;
~Logger(){}
public:
Logger(bool addRef = false) : m_ref(0)
{
if(addRef) m_ref++;
}
void addRef()
{
m_ref++;
}
void release()
{
if(--m_ref == 0)
delete this;
}
void write(const char * text)
{
cout << text << endl;
}
};
template<typename T>
class CountingPtr
{
T * m_t;
public:
// wrap existing ptr
CountingPtr(T * t, bool addRef = true) : m_t(t)
{
assert(t!=0);
if(addRef)
t->addRef();
}
// release my ref
~CountingPtr()
{
if(m_t)
m_t->release();
}
// assing me
CountingPtr(const CountingPtr & rhs) : m_t(0)
{
*this = rhs;
}
// release prev ptr, get new one
// self assignment considered error
CountingPtr&operator=(const CountingPtr & rhs)
{
if(this == &rhs)
assert(false); // consider it is bad
else
{
if(m_t)
m_t->release();
m_t = rhs.m_t;
m_t->addRef();
}
return *this;
}
T * operator->()
{
return m_t;
}
};
typedef CountingPtr<ILogger> CLoggerPtr;
void use(CLoggerPtr logger)
{
logger->write("use");
}
class Loggable
{
public:
Loggable(CLoggerPtr logger) : m_logger(logger)
{}
private:
CLoggerPtr m_logger;
};
class X : public Loggable
{
public:
X(CLoggerPtr logger)
try : Loggable(logger) // comment try block out to
get working code!
{}
catch(std::runtime_error & )
{
throw;
};
};
int main(int argc, char* argv[])
{
try
{
CLoggerPtr logger(new Logger());
use(logger);
{
X y1(logger);
}
{
X y2(logger); // OOPS IS HERE! If try block is in act, program tries
to reference dangling pointer
}
return 0;
}
catch(exception & ex)
{
cout << "Exception: " << ex.what() << endl;
return EXIT_FAILURE;
}
catch(...)
{
cout << "(...) exception" << endl;
return EXIT_FAILURE;
}
}