DevShiftTeam / AppShift-MemoryPool

A very fast cross-platform memory pool mechanism for C++ built using a data-oriented approach (3 to 24 times faster than regular new or delete, depending on operating system & compiler)
Apache License 2.0
214 stars 25 forks source link

Patch memory release #23

Closed fysu closed 1 year ago

fysu commented 2 years ago

这里需要判读下是否是要被释放的块,因为当前块要被释放了,作者没有清理,导致下次分配内存的时候就会出现问题,这里看下我的修复

LessComplexity commented 1 year ago

@fysu

这里需要判读下是否是要被释放的块,因为当前块要被释放了,作者没有清理,导致下次分配内存的时候就会出现问题,这里看下我的修复

Thank you for the fix, your fix is still not correct but it addresses a real issue, the correct fix for it is to change the first 2 conditions:

// Case of the first block
if (block == this->firstBlock) {
    this->firstBlock = block->next;
    this->firstBlock->prev = nullptr;
}
// Case of the last block
else if (block == this->currentBlock) {
    this->currentBlock = block->prev;
    this->currentBlock->next = nullptr;
}

The reason your proposed fix poses a problem can be seen for the first case:

if (this->firstBlock->next == block) {
    this->firstBlock->next = 0;
}

What if block is between firstBlock and currentBlock? then the first block will not be linked to the last block and it will cause a problem when freeing the pool. Same goes for the condition:

if (this->currentBlock->prev == block) {
    this->currentBlock->prev = 0;
}

The other conditions are correct and they solve the problem, I just "integrated" them into the first 2 conditions that are already present.

Thank you very much! :)