_crtisvalidheappointer(block) when deleting base class pointer

Sorry for the long post, but i need help in understanding this.

I have below code which throws _crtisvalidheappointer(block)

#include <iostream>
#include <string>
using namespace std;

#define ASSERT_ERROR
/*#define SOLVE_ASSERT*/

class Player
{
    std::string playerName;
public:
    Player(std::string &playerName) :playerName(playerName) { cout << "Player Constructed\n"; }
    void printPlayerName() const
    {
        std::cout << playerName << endl;
    }
#if defined SOLVE_ASSERT
    virtual ~Player() { cout << "Player Destructed\n"; }
#else
    ~Player() { cout << "Player Destructed\n"; }
#endif
};
#if defined ASSERT_ERROR
class Batsman : virtual public Player
#else
class Batsman : public Player
#endif
{
public:
    Batsman(std::string playerName) : Player(playerName) { cout << "Batsman info added\n"; }
    ~Batsman() { cout << "Batsman Destructed\n"; }
};

int main()
{
    Player *ptr = new Batsman("Sachin Tendulkar");
    ptr->printPlayerName();
    delete ptr;
}

As soon as i uncomment below line i get this issue resolved.

#define SOLVE_ASSERT

I referred below link where it said single inheritance could not cause an issue. https://stackoverflow.com/a/48792493/8679501

But in my case i just used virtual to inherit the class which is kind of mandating me to make destructor virutal. why is it so?

Why below line of code is legal without virtual inheritance and why not with virtual inheritance?

Player *ptr = new Batsman(playerName);

Solution

Before I start, here is a slightly modified version of your code that I used to debug this:

#include <iostream>
#include <string>
using namespace std;

class Player {
    std::string playerName;
public:
    Player(std::string &playerName) :playerName(playerName) { 
        cout << "Player Constructed @" << this << endl; 
    }
    
    void printPlayerName() const {
        std::cout << playerName << endl;
    }
    
    ~Player() { 
        cout << "Player Destructed @" << this << endl; 
    }

};

class Batsman : virtual public Player {
public:
    Batsman(std::string playerName): 
        Player(playerName)
    { 
        cout << "Batsman info added @" << this << endl; 
    }
    
    ~Batsman() { 
        cout << "Batsman Destructed @" << this << endl; 
    }
};

int main() {
    Player *ptr = new Batsman("Sachin Tendulkar");
    ptr->printPlayerName();
    delete ptr;
}

If we try to run your code in this current state, we get the following output message (though the pointer values may vary):

Player Constructed @0x1048c68                                                                                                                                           
Batsman info added @0x1048c60                                                                                                                                           
Sachin Tendulkar                                                                                                                                                        
Player Destructed @0x1048c68                                                                                                                                            
*** Error in `./a.out': free(): invalid pointer: 0x0000000001048c68 ***                                                                                                 
Aborted (core dumped)

If we look at the pointer values, we notice that the memory is being freed in the wrong location. For another example of this, consider the following code:

int* arr = new int[100];
cout << "Array is @" << arr << endl << "Memory being freed is @" << (arr+50) << endl;
delete[] (arr+50);

If we run this, we get a very similar output to what we got in your example:

Array is @0xd8cc20                                                                                                                                                      
Memory being freed is @0xd8cce8                                                                                                                                         
*** Error in `./a.out': free(): invalid pointer: 0x0000000000d8cce8 ***                                                                                                 
Aborted (core dumped)

We are getting this error because the 200 bytes between 0xd8cc20 and 0xd8cce8 are not actually being freed, creating a memory leak. Going back to your program, we notice that the bytes between 0x1048c60 and 0x1048c68 aren't being freed. In order to fix this, we must ensure that the correct destructor is called, which we can do in one of two ways:

We can declare ptr to be a Batsman* instead of a Player*. This will tell the compiler to call the Batsman destructor rather than the Player constructor, which successfully frees all the memory.
We could make the ~Player() function virtual. This indicates to the compiler that the correct functions to be called will be determined at runtime. If this is done, the Batsman destructor will be called, even though the object was declared as a pointer to a Player.

EDIT: Here is the explanation to why removing virtual from class Batsman : virtual public Player seems to fix the error:

When you are using virtual inheritance, the subclass will allocate its memory before the superclass. As a result, the point at which the superclass starts allocating its memory will be well after the actual start of the object. This explains why in your example, the Batsman object was allocated 8 bytes the before the Player was. When free() tries to deallocate using the superclass address, it will see that no object was allocated there, causing an error.

Meanwhile, in normal inheritance, both the subclass and the superclass will start allocating at the same address. This means that you can free the memory using the superclass address. Since there was a pointer allocated at the superclass's address, the C++ memory management system acts as though it is a valid deallocation.