Bugs causing memory leak and alloc-dealloc-mismatch

[Sweetlemon68]

During my test with AddressSanitizer enabled, the sanitizer reports that there are memory leaks and alloc-dealloc-mismatch in the codes. Using stack trace provided by ASan, the bugs are found (one of them is hard to solve).

The bug that causes memory leak is at PageManager::Free in storage/page-manager.cpp. Before removing the page information in the unordered map, we should delete its corresponding buffer.

// In PageManager::Free
eviction_policy_.Remove(pgid);
auto it = buf_.find(pgid);
if (it != buf_.end()) {
  assert(it->second.refcount == 0);
  delete[] it->second.addr; // We may add this line.
  buf_.erase(it);
}

The bugs that cause alloc-dealloc-mismatch are at:

• Line 43 of common/bitvector.hpp, where mem_ is mem_ = std::unique_ptr<uint64_t>(new uint64_t[_get_alloc_size(size_)]);. Note that if we change the type here, there is also a need to modify the corresponding definition of mem_. (Change the type of mem_ to std::unique_ptr<uint64_t[]>.)
• Line 24 of type/vector.hpp, We might change the type of str_ as std::unique_ptr<uint8_t[]>.
• Line 17 of common/allocator.hpp. We might change the type of ptrs as std::vector<std::unique_ptr<uint8_t []>>.

And there is also a bug causing alloc-dealloc-mismatch which is too complex for me to find a possible fix. In Line 20 of type/static_field.hpp, a memory block is allocated with new[] but immediately cast to StaticStringField *, and later during destruction of vector(), this is freed using delete. This is due to the type of smart pointer is not completely matched with the allocator, and I cannot figure out a solution.

After figuring out the last bug, I turned off the alloc-dealloc-mismatch check of ASan, so there may be other such bugs.

Thanks for attention :)

[seekstar]

Thank you for your feedback! You are right about the memory leak in PageManager::Free. It seems that holding ownership of a memory area with a raw pointer is a bad idea. So I fixed the memory leak by holding the ownership of buffer pages with std::unique_ptr. You may check it again with AddressSanitizer, which I'm not familiar with. The related code of rest issues is actually written by @yfzcsc. I'll check them later.

[seekstar]

Types of mem_, str_, and ptrs are also fixed now. As for the last issue, I didn't see the destruction of vector() in destruction of vector().

[Sweetlemon68]

Types of mem_, str_, and ptrs are also fixed now. As for the last issue, I didn't see the destruction of vector() in destruction of vector().

I just checked for the underlying error of the last issue. In Line 20 of type/static_field.hpp, the memory allocated using new uint8_t[] is cast to StaticStringField* and returned. Then in Line 317 of execution/exprdata.cpp, this memory is managed by std::shared_ptr<StaticStringField>, so it will be released using delete, which causes the mismatch. This cannot fixed by a simple modification, and may require to modify the structure of StaticStringField.

[seekstar]

@yfzcsc Maybe you should fix the last issue mentioned here. I'm not familiar with the related code.

[yfzcsc]

Fixed in https://github.com/iidb/wing/commit/8cf2c85ccb41829be639280a68517ce18346d7d3.