Strange error

jacobnavia

unread,

Sep 24, 2015, 1:09:18 PM9/24/15

to

ForwardPool.cpp:97:4: Delete called on 'CForwardBlock' that is abstract
but has non-virtual destructor

???
That looks like chinese to me.

What could be all about?

while(true)
{
CForwardBlock* p=m_disk_pool.Get();

if(p==NULL) {break;}
else
{
InterlockedDecrement(&m_disk_pool_size);
p->freeit();
delete p; <<<<<<<<<<<<<here
}
}

The definition of that class looks like this

class CForwardBlock
{
char* p_buf;
mlong m_size; //this is the current size of buffer
mlong m_pos; //this is the current position in buffer
mlong m_max_size;

public:

CForwardBlock(mlong size)
{
m_max_size=size;
m_pos=0;
m_size=0;

}
void init() {p_buf = getBuffer(m_max_size);}

void freeit() {freeBuffer(p_buf);}
~CForwardBlock()
{
}

// ...

Melzzzzz

unread,

Sep 24, 2015, 1:29:20 PM9/24/15

to

It should have at least one pure virtual function. Deleting through
pointer to such class is undefined behavior if destructor is not
virtual, I think. Good thing is that you got compiler error instead...

Melzzzzz

unread,

Sep 24, 2015, 1:34:49 PM9/24/15

to

To be more precise deleting anything through pointer to base class is
undefined if destructor in base class is not virtual.

bartekltg

unread,

Sep 24, 2015, 2:45:26 PM9/24/15

to

Do you have here any virtual function? If yes, you probably
want to write delivered class, and use an object of this class...
using polymorphism (I mean by base-class pointer). In this case,
only the destructor of the base class will be called.
To avoid this your destructor should be virtual.

http://stackoverflow.com/questions/1123044/when-should-your-destructor-be-virtual

bartekltg

>

Martin Shobe

unread,

Sep 24, 2015, 2:47:55 PM9/24/15

to

On 9/24/2015 12:09 PM, jacobnavia wrote:
> ForwardPool.cpp:97:4: Delete called on 'CForwardBlock' that is abstract
> but has non-virtual destructor
>
> ???
> That looks like chinese to me.

An abstract class is one that contains at least one pure virtual
function. The important consequence here is that it can only be created
as the base class of another class. When you destroy an instance of a
class, the destructor called needs to be the one for the most derived
class. In this case, because it's abstract, it can't be CForwardBlock.
Since the destructor is not virtual, the destructor for CForwardBlock is
called and not the one that should be.

To fix it, you should make the destructor virtual.

Martin Shobe

jacobnavia

unread,

Sep 24, 2015, 3:09:58 PM9/24/15

to

Thanks for all these answers. I will try to fix this by doing as
required: make a virtual destructor.

That would imply that all classes that inherit from it should define a
destructor... Will the compiler warn me if there is a class that
inherits from that class and hasn't defined its destructor?

Thanks to all

Christopher Pisz

unread,

Sep 24, 2015, 3:15:54 PM9/24/15

to

On 9/24/2015 12:09 PM, jacobnavia wrote:

Consult the documentationfor CForwardBlock, whatever that is.
It looks like some MFC-esk poopy written by someone who has no notion of
RAII.

The error is coming from some part of CFowardBlock that you left out. It
probably has a pure virtual function and was made to be derived from.

Why in the world you need a seperate init and freeit method is not
apparent. Why not allocate in the constructor and release in the
deconstructor? This class will leak as is, if an exception is thrown.

You also didn't show what m_disk_pool's type is or what it's get method
returns.

--
I have chosen to troll filter/ignore all subthreads containing the
words: "Rick C. Hodgins", "Flibble", and "Islam"
So, I won't be able to see or respond to any such messages
---

Martin Shobe

unread,

Sep 24, 2015, 3:25:58 PM9/24/15

to

No, because the compiler will supply a default one. You should only need
to define a destructor for those classes that need to do something other
than just destroy it's sub-objects.

Martin Shobe

Melzzzzz

unread,

Sep 24, 2015, 3:44:00 PM9/24/15

to

You don't have to define destructor in derived classes.

>
> Thanks to all

jacobnavia

unread,

Sep 24, 2015, 5:00:37 PM9/24/15

to

Le 24/09/2015 21:15, Christopher Pisz a écrit :

> Consult the documentationfor CForwardBlock, whatever that is.

That's very easily done. Thanks. No documentation.

> It looks like some MFC-esk poopy written by someone who has no notion of
> RAII.
>

Maybe, if you say so, I would agree if I knew what "MFC-esk" could mean
in a Unix environment with gcc as compiler...

> The error is coming from some part of CFowardBlock that you left out. It
> probably has a pure virtual function and was made to be derived from.
>

It has many virtual functions, yes.

> Why in the world you need a seperate init and freeit method is not
> apparent. Why not allocate in the constructor and release in the
> deconstructor? This class will leak as is, if an exception is thrown.
>

Yes, I thought C++ would use that but the people that wrote that didn't.
I can't tell you their reasons, and your reasons look reasonable at
first sight. But first sights do not count here. Maybe there was a
reason, I do not know.

> You also didn't show what m_disk_pool's type is or what it's get method
> returns.
>
>

It is a Fifo template,

template<class T, class P, int LenLn2>
class FifoBase

and one of the template's method is

...
T *Get();

Mmmmm that brings us not a lot more near the solution but...

Christopher Pisz

unread,

Sep 24, 2015, 5:23:09 PM9/24/15

to

On 9/24/2015 4:00 PM, jacobnavia wrote:
> Le 24/09/2015 21:15, Christopher Pisz a écrit :
>
>> Consult the documentationfor CForwardBlock, whatever that is.
>
> That's very easily done. Thanks. No documentation.
>
>> It looks like some MFC-esk poopy written by someone who has no notion of
>> RAII.
>>
> Maybe, if you say so, I would agree if I knew what "MFC-esk" could mean
> in a Unix environment with gcc as compiler...

I mean naming classes "C<somename>" as if the prefix of C does something
special for it. CFile, CSocket, CSnuffleupogus, etc. They also love
manual reference counting that makes bugs. Maybe the practice came from
somewhere else before MFC, I dunno, but it is silly. You'll also see
increment this and decrement that all over the place, usually without
concern for what situations it will actually increment to decrement
correctly...which brings us to RAII. Write a test in your compiler where
you create something that is a CFowardBlock and throw an exception after
you create it. Put a breakpoint on the call that frees the buffer. Did
it get freed or did it leak?

>
>> The error is coming from some part of CFowardBlock that you left out. It
>> probably has a pure virtual function and was made to be derived from.
>>
>
> It has many virtual functions, yes.
>
>> Why in the world you need a seperate init and freeit method is not
>> apparent. Why not allocate in the constructor and release in the
>> deconstructor? This class will leak as is, if an exception is thrown.
>>
>
> Yes, I thought C++ would use that but the people that wrote that didn't.
> I can't tell you their reasons, and your reasons look reasonable at
> first sight. But first sights do not count here. Maybe there was a
> reason, I do not know.
>
>> You also didn't show what m_disk_pool's type is or what it's get method
>> returns.
>>
>>
>
> It is a Fifo template,
>
> template<class T, class P, int LenLn2>
> class FifoBase
>
> and one of the template's method is
>
> ...
> T *Get();
>
>
> Mmmmm that brings us not a lot more near the solution but...
>

Anand Hariharan

unread,

Oct 5, 2015, 7:00:00 PM10/5/15

to

On Thursday, September 24, 2015 at 4:00:37 PM UTC-5, jacobnavia wrote:
> Le 24/09/2015 21:15, Christopher Pisz a écrit :
> > The error is coming from some part of CFowardBlock that you left out. It
> > probably has a pure virtual function and was made to be derived from.
> >
>
> It has many virtual functions, yes.
>

Presumably, one or more of those virtual member functions have "= 0;" at the end of their declarations? That is what makes CForwardBlock 'abstract' (i.e., it cannot be instantiated on its own).

- Anand