Deleting caller object safely within a callback function

Question

Code segment from the library:

class Client{
public:
    class CallBack {
    public:
        virtual void onData(Client* caller, std::string& data) =0;
    };

    Client(CallBack* callback):m_callBack(callback){}
    virtual ~Client(){}
    void onData(std::string data) {
        m_callBack->onData(this, data);
        m_totalDataVol += data.size();
    }

private:
    CallBack* m_callBack;
    int m_totalDataVol = 0;
}

Code segment from the application:

class AppHandler: public Client::Callback {
    void onData(Client* caller, std::string& data) {
        /* Some complex logic and check certain conditions*/
            delete caller; // Application will crash, due to 
                           // accessing member of deleted object (m_totalDataVol)
    }
}

In addition Caller object (instance of Client class) is owned by the application, and Application no restriction to delete that.

How do I overcome this problem?

Very complex scenario: Client class of the base library can be extended by a another library (ClientEx class) and the application might use that extended library (Not the base library)

Answer 1

Have your callback return a bool indicating whether the caller should delete itself. Do not delete the client from the callback.

Also, does the callback need to be called at all if data.size == 0? The Client could check for this condition before calling the callback, and delete itself (or otherwise handle it appropriately).

If the callback still does need to be called, perhaps you can avoid changing the return type just by checking the condition in the client after the call:

void onData(std::string data) {
    m_callBack->onData(this, data);
    if (data.size() != 0) {
        m_totalDataVol += data.size();
    }
    else {
        delete this;
    }
}

Alternatively, if you really must allow the callback to delete the client, then you need some way of tracking when the client has been deleted which you can use within the client itself. This means keeping a reference to another object:

class Client{
public:
    class CallBack {
    public:
        virtual void onData(Client* caller, std::string& data) =0;
    };

    Client(CallBack* callback):m_callBack(callback){}, was_deleted(nullptr)
    virtual ~Client(){
        if (was_deleted) *was_deleted = true;
    }
    void onData(std::string data) {
        bool *was_deleted = new bool();
        this->was_deleted = was_deleted;
        m_callBack->onData(this, data);
        if (! *was_deleted) {
            m_totalDataVol += data.size();
            this->was_deleted = nullptr;
        }
        delete was_deleted;
    }

private:
    CallBack* m_callBack;
    int m_totalDataVol = 0;
    // When issuing a callback, holds a pointer to a flag that can
    // be used to track if this object has been deleted:
    bool * was_deleted;
}

(Note that the solution above is not thread-safe, but could probably be made so. Also note the code above doesn't compile, just as the sample code in your question doesn't - I have tried to match the original code as well as possible; the principle should be applicable to real code).