开发者

Where should I free the object?

开发者 https://www.devze.com 2023-03-14 09:19 出处:网络
void ServerManager::addWork(SocketClient *clientSocket, vector<char> data){ Work *work = new Work(clientSocket, data);
void ServerManager::addWork(SocketClient *clientSocket, vector<char> data){
    Work *work = new Work(clientSocket, data);
    m_workHandler->addWork(work);
}

Here I created Work instance and hand the work to the WorkHandler.

m_workHandler->addWork(work);

This code is inserting the work into the queue.

This is one part of my server and if I run this code for long time then the server crashes with error saying like this

what() std::bad_alloc

I looked online and it is an error message that show there isn't memory space available. So I realized I didn't free the work object after I am done with usin开发者_开发技巧g it.

So I was wondering where I should free the work object.

void ServerManager::addWork(SocketClient *clientSocket, vector<char> data){
    Work *work = new Work(clientSocket, data);
    m_workHandler->addWork(work);
    delete work;
}

Can I do this? If I pass the work instance then it increases the reference count?

void WorkHandler::workLoop(){
    printf("WorkHandler::workLoop, called\n");

    while(m_workHandlerRun){
        Work *work = getWork();
        char *pdata = work->getVPointer();

        unsigned char type = pdata[0];

        printf("WorkHandler::workLoop, type %d\n", type);

        Packet *packet = m_packetFactory->createInstance(static_cast<PACKET_TYPES>(type));
        packet->fromByte(pdata);

        delete work;
    }
}

This is the code I handle the work....Should I delete work instance here or both place? Thanks in advance...


Judging by your second bit of code, it looks like the memory leak might actually be from the Packet allocation, because the Work instance is being deleted in what seems like a correct place (in workLoop, not addWork). But I can't be certain without seeing more of your code.

Allocations in C++ don't have a reference count, so you should delete the Work instance when you're done using it. If you delete it beforehand, it's gone: use that pointer and the results will be undefined (might work, probably won't). If you don't delete it, it will leak (as you can see).

You should refactor your code to use smart pointers -- look at std::unique_ptr<> and std::shared_ptr<> (or boost::scoped_ptr<> and boost::shared_ptr<>). They will help ensure things get deleted when they really should be:

for(;;)
{
    std::unique_ptr<int> uptr(new int);
    std::shared_ptr<int> sptr = std::make_shared<int>();

    // uptr and sptr will be automatically deleted at this end of scope.
}


If you are inserting the work into a queue and that isn't done processing yet, you don't want to delete it right after adding it in.

What you want to do is carefully look at your code and when you no longer need the object around you delete it. This generally will be after you are done working on it in your WorkHandler and it has been safely removed from the queue.


Deleting in addWork() won't do - you've just added the object and some other code might start accessing it. If you delete it now you run into undefined behavior. workLoop() is a much better place - you've done all the work on the object and looks like it is no longer needed, so you can delete it here.


You should free the object (delete) once you're done with it. You shouldn't delete the same object twice. Once you've delete an object, you should never try to access it via some other pointer.

Remember, C++ doesn't have any garbage collection. So there is no reference counting. If you go for new and delete, you need to manually handle allocation and deallocation yourself. Otherwise, you can use auto_ptr.


  1. If you delete work in first place you will, most probably, get crash trying to read from pointer which doesn't point to correct object, but standard says that it is UB, so it is possible that your program will not crash.
  2. If you delete work in second place it is ok.

But the better solution is placing work to some smart pointer, which will automatically delete it after destroying (in your case after leaving while loop in workloop). Use boost::shared_ptr<> for that.


No, that is disastrous. work pointer is still in use and you are deleteing it. Which makes it a dangling pointer. Referring a dangling pointer is undefined behavior.

Use shared_ptr<> or other smart pointer if you don't want to manage the freeing of the object. Or else, delete them inside ServerManager::~ServerManager() as you will still have their references inside m_workHandler.

0

精彩评论

暂无评论...
验证码 换一张
取 消