aggressive decommit mode alloc may trigger core in 2.8.0

[qhsong]

I want to alloc large memory in aggressive decommit mode, but it trigger an coredump:

#0  std::_Rb_tree_increment (__x=0x0)
    at ../../../.././libstdc++-v3/src/c++98/tree.cc:91
#1  0x00000000004535af in std::_Rb_tree_const_iterator<tcmalloc::SpanPtrWithLength>::operator++ (this=0x7fffffffdd30)
    at /usr/local/gcc-4.9.4/include/c++/4.9.4/bits/stl_tree.h:283
#2  0x00000000004539fc in std::_Rb_tree<tcmalloc::SpanPtrWithLength, tcmalloc::SpanPtrWithLength, std::_Identity<tcmalloc::SpanPtrWithLength>, tcmalloc::SpanBestFitLess, tcmalloc::STLPageHeapAllocator<tcmalloc::SpanPtrWithLength, void> >::erase[abi:cxx11](std::_Rb_tree_const_iterator<tcmalloc::SpanPtrWithLength>) (
    this=0x843cf8 <tcmalloc::Static::pageheap_+1572920>, __position=...)
    at /usr/local/gcc-4.9.4/include/c++/4.9.4/bits/stl_tree.h:856
#3  0x0000000000453543 in std::set<tcmalloc::SpanPtrWithLength, tcmalloc::SpanBestFitLess, tcmalloc::STLPageHeapAllocator<tcmalloc::SpanPtrWithLength, void> >::erase[abi:cxx11](std::_Rb_tree_const_iterator<tcmalloc::SpanPtrWithLength>) (
    this=0x843cf8 <tcmalloc::Static::pageheap_+1572920>, __position=...)
    at /usr/local/gcc-4.9.4/include/c++/4.9.4/bits/stl_set.h:591
#4  0x0000000000451b4b in tcmalloc::PageHeap::RemoveFromFreeList (
    this=0x6c3cc0 <tcmalloc::Static::pageheap_>, span=0x84f1e0)
    at src/page_heap.cc:445
#5  0x0000000000451635 in tcmalloc::PageHeap::CheckAndHandlePreMerge (
    this=0x6c3cc0 <tcmalloc::Static::pageheap_>, span=0x84f1b0, other=0x84f1e0)
    at src/page_heap.cc:349
---Type <return> to continue, or q <return> to quit---
#6  0x0000000000451894 in tcmalloc::PageHeap::MergeIntoFreeList (
    this=0x6c3cc0 <tcmalloc::Static::pageheap_>, span=0x84f1b0)
    at src/page_heap.cc:392
#7  0x0000000000451d25 in tcmalloc::PageHeap::ReleaseSpan (
    this=0x6c3cc0 <tcmalloc::Static::pageheap_>, span=0x84f1b0)
    at src/page_heap.cc:511
#8  0x00000000004515e8 in tcmalloc::PageHeap::CheckAndHandlePreMerge (
    this=0x6c3cc0 <tcmalloc::Static::pageheap_>, span=0x84f1e0, other=0x84f1b0)
    at src/page_heap.cc:339
#9  0x000000000045176b in tcmalloc::PageHeap::MergeIntoFreeList (
    this=0x6c3cc0 <tcmalloc::Static::pageheap_>, span=0x84f1e0)
    at src/page_heap.cc:382
#10 0x000000000045156e in tcmalloc::PageHeap::Delete (
    this=0x6c3cc0 <tcmalloc::Static::pageheap_>, span=0x84f1e0)
    at src/page_heap.cc:318
#11 0x0000000000452634 in tcmalloc::PageHeap::GrowHeap (
    this=0x6c3cc0 <tcmalloc::Static::pageheap_>, n=256)
    at src/page_heap.cc:699

tcmalloc alloc 0x84f1e0 span and then delete it, in aggressive decommit mode, it will try merge prev span 0x84f1b0, so call ReleaseSpan(0x84f1b0).
But ReleaseSpan(0x84f1b0) will call RemoveFromFreeList(0x84f1e0), it is a new span and iter is empty, so erase it may core.

This is my test source code:

  MallocExtension::instance()->SetNumericProperty("tcmalloc.aggressive_memory_decommit", 1);
  std::vector<void *> ptr_vec;
  for(int count = 0; count < 10; ++count)
  for(int i = 0; i < 512; i++)
  {
      size_t size = 256 * 8 * 1024;
      void *ptr = malloc(size);   //Core here and i=0
      if(ptr == nullptr)
      {
        std::cout<<"can not alloc memory"<<std::endl;
        continue;
      }
      memset(ptr, 1, size);
      ptr_vec.push_back(ptr);
  }

  std::cout<<"memory alloc: "<<ptr_vec.size()<<std::endl;

[alk]

Much thanks for high quality bug report and also thanks for the patch. This is actually even trickier a bit. And I think your patch doesn't fully fix the problem.

The problem is that after we decommited our "main" span that is being deleted, we mark it as sitting on freelist, where in fact it isn't properly inserted there yet. Then when we run decommit for adjacent spans, we drop lock. And while that lock is dropped other operations may "see" our original span and merge with it, corrupting memory in the process.

I think better fix would be to just not bothering with decommitting adjacent spans at all. We will rarely face adjacent spans not on returned free list anyways. And all we need from aggressive decommit is dealing with memory we're freeing. All this "adjacent spans" business is not important and tricky to get right.

There also another reports of issues with problematic commit. Which may be caused by same bug, or maybe some other bug. I looked through the code and I think it there should be no other bugs. But in any case I've asked if those other reports also do aggressive decommit. For now I have reverted problematic commit.

[qhsong]

Got it, I'm not fully understand how span release work and do some tricky patch.
Maybe I can pull a test case on this situation, I found aggressive decommit mode do not have enough test cases.

I will close this issues.
Thanks for your reply