Deleting objects in callback function

Question

I am sending an HTTP request to save/update data on the server. The request is made asynchronously and a callback function is called when it completes. Everything works fine except that sometimes, the application crashes in the callback.

This is what I am doing:

user = new User();
user->saveOnServer();
user->zombie = true;  // Mark the user that it needs to be deleted in the callback.

In User, I have a saveOnServer() method:

void User::saveOnServer(){
    Request *request = new Request();

    // Send request to the server and register the callback.
    request ->setCallback(&userCallback, (void*)this);
}

The callback:

void userCallback(void *data){
    User *user = (User*)data;

    // Do something here.
    // Delete user if it's a zombie.
    if(user->zombie)
        delete user;
}

At times, I need to create a new user after sending a request to the server:

user = new User();
user->saveOnServer();
user->zombie = true;
// Some code comes here.
if(user)
    delete user;
user = new User();

The problem is that in such cases, the application crashes when deleting the user in the callback as it has already been deleted. Another issue is that the callback deletes user but user pointer in main is still pointing to some address (dangling pointer) and so I again try to delete it.

I am not sure what is the best way of managing memory in this case. I have zombie in place because there are cases when I do not want the callback to delete the user.

Answer 1

Once you've called saveOnServer of a zombie user, the request is the effective "owner" of that user object. Don't free it yourself since there's something else that still intends to use it and delete it later.

In fact, if the server action can return asynchronously, then the user object might get destroyed at any time. You should cease using it entirely from the other code. You've granted control of that object to the request, and you must stop using it from anywhere else:

user = new User();
user->zombie = true; // set *before* transferring ownership to server
user->saveOnServer();
user = NULL;
//some code comes here
user = new User();

If you don't want the request to use that object anymore, then you need to provide some facility for "canceling" the save-on-server action so that it doesn't use the object.

---

Another option is to use smart pointers. In your main code, store the object in a shared_ptr. In the request object, store it in a weak_ptr. That way, if your main code wants to destroy the user object, it can simply call user.reset(). Then if the callback attempts to use the weak_ptr, it will discover that the pointed-to object is no longer available. When using smart pointers, neither function should use delete. The pointer objects will manage the lifetime of the user for you.

shared_ptr<User> user = make_shared<User>()
user->saveOnServer();
//some code comes here
user.reset(new User());

In the saveOnServer function, use shared_from_this to create a weak_ptr to the object:

void User::saveOnServer(){
  Request *request = new Request();

  //send request on server and register the callback
  weak_ptr<User> self(shared_from_this());
  request ->setCallback(&userCallback, self);
}

In the callback, use that weak_ptr:

void userCallback(weak_ptr<User> data){
  shared_ptr<User) user = data.lock();
  if (!user)
    return;

  //do something here
}