0

Context

I have my own implementation of binary heap with decrease-key functionality. I'm using it on my Contraction Hierarchies implementation. When initializing a BinaryHeap, all works normally, but after the construction finishes, all the values of the BinaryHeap are setted to 0 (or 0 like values) and thus heap property no longer holds. I think it could be related to the usage of new method

Details of the error

In BinaryHeap class, the variable that holds the information of the heap is items_, a vector of pointer to Items<payload=int, value=float>. When calling BinaryHeap(pairs, handles) from a vector of pairs (62 elements) with negative and positive values, items_ is correctly setted inside the constructor, and check() (check heap property) runs perfectly. Outside the constructor, I called check() inmediately after, and it fails (heap property no longer holds). All values are now 0 or 0-ish (3e-41) and then the comparision assert(!less(index, parent)) fails (for some item, 0 < 3e-41). The vector items_ is still size 62.

Things I've tried:

  • With gdb, I checked the variables of the binary heap, and items_ is still a valid vector with 62 items, but all values are 0 or 0-ish values (3e-41). No memory leaks, but the information of the heap is lost.
  • If I run this binary heap implementation on an isolated test environment, this doesn't happen. I'm not including the whole BinaryHeap implementation, but I've tested it in all the basic cases and it works fine with no memory leaks.
template <class Payload, class Value>
struct Item{
    Payload id;
    Value value;
    int index;
    Item(Payload id=-1, Value value=10, int index=1) : id(id), value(value), index(index) {};

};

template <class Payload, class Value>
class BinaryHeap {
public:
    BinaryHeap(){
        items_.resize(1);
        }
    // Builds a BinaryHeap from a vector of pairs
    BinaryHeap(vector<std::pair<Payload, Value>> pairs, vector<Item<Payload, Value>*>& handles)
    {
        items_.resize(pairs.size()+1);
        
        for (size_t i = 0; i < pairs.size(); i++)
        {
            items_[i+1] = new Item<Payload, Value>(pairs[i].first, pairs[i].second, i+1);
            handles[i] = items_[i+1];
        }
        for (size_t i = items_.size()/2; i > 0  ; i--)
        {
            sink_down(i);
        }
        // works fine
        check();    
    }
    
    void check() const {
        // check if heap property holds
        if (size() > 1)
        {
            for (int index = 2; index <= size(); index++) {
                int parent = parentOf(index);
                assert(!less(index, parent));
                assert(items_[index]->index == index);
            }
        }
    }
    
    int size() const {
        return items_.size()-1;
    }

    private:
        std::vector<Item<Payload, Value>*> items_;
    bool less(int i1, int i2) const {
        return items_[i1]->value < items_[i2]->value;
    }

    // Contraction hierarchies implementation, pqueue is a BinaryHeap.
    pqueue = BinaryHeap(pairs, handles_CH);
    // raises error
    pqueue.check();
BMarin
  • 55
  • 6
  • 1
    Can we get a [mre]? Also, in `check`, it looks like you access out of bounds with `i==size()`, and the second `assert` is _assigning_, not _comparing_. – 1201ProgramAlarm Apr 23 '21 at 19:37
  • Size of the heap is the size of the vector minus 1, thus size() is not out of bouds (I'm including it). About the second assert, you're right, i've fixed it, but it doesn't raises any error. I need some time to get a minimal example. – BMarin Apr 23 '21 at 19:41
  • 1
    ... and the whole thing leaks memory. And there's evidence of forced 1-based array indexing, which always ends in tears. This whole thing needs to be redesigned. – Sam Varshavchik Apr 23 '21 at 19:43
  • From https://en.wikipedia.org/wiki/Binary_heap "Specifically, sometimes the root is placed at index 1, in order to simplify arithmetic." – BMarin Apr 23 '21 at 19:45
  • This is still lacking. You don't show the destructor or assignment operators. It is possible some combination of those is resulting in use-after-free and/or a double delete. – 1201ProgramAlarm Apr 23 '21 at 23:38

0 Answers0