Segmentation Fault In Recurisve Structure With Smart Pointer
A singly linked list implemented with smart pointer, which has a reference to next, can easily cause Stack Overflow at its destruction because the destructor of the next, and the next, and the next, are called recursively.
Here's the code.
Code:
#include <boost/shared_ptr.hpp> | |
#include <iostream> | |
| |
#define RECURSIVE | |
| |
class IntSLLNode { | |
| |
private: | |
int index; | |
boost::shared_ptr<IntSLLNode> next; | |
| |
IntSLLNode(int ix){ | |
index = ix; | |
} | |
| |
public: | |
int hasNext(){return next != 0;} | |
boost::shared_ptr<IntSLLNode>& moveNext(){return next;} | |
int getIndex(){return index;} | |
friend class IntSLList; | |
}; | |
| |
class IntSLList { | |
private: | |
boost::shared_ptr<IntSLLNode> head, tail; | |
public: | |
~IntSLList(){ | |
#ifndef RECURSIVE | |
while(head != tail){ | |
// replace | |
head = head->next; | |
} | |
#endif | |
} | |
| |
void addNode(int index){ | |
| |
boost::shared_ptr<IntSLLNode> n(new IntSLLNode(index)); | |
if(head == 0){ | |
head = tail =n; | |
} | |
else | |
{ | |
tail->next = n; | |
tail = n; | |
} | |
| |
} | |
| |
boost::shared_ptr<IntSLLNode>& getHeadNode(){return head;} | |
}; | |
| |
int main(){ | |
IntSLList list; | |
// the number to reproduce crush may vary, but seems to be stable | |
// On my Core2Duo box, it is around 65000 | |
// Here I dare to use a value large enough | |
for(int i=0; i<1000000; i++){ | |
list.addNode(i); | |
} | |
// crush at destruction | |
std::cout << "CRUSH?" << std::endl; | |
} |
This works by commenting out the line 4.
For your own smart pointer, the point is line 7 in the following code.
Code:
SmartPointer& operator=(const SmartPointer& src){ | |
if(this != &src){ | |
if(counter->decrement() == 0){ | |
delete counter; | |
//src.pointee_ = 0; // NOT DESTRUCTIVE | |
counter = src.counter; | |
counter->increment(); // increment before deleting pointee_, which might have a reference to src. This results in a temporal no referencer, and triggers recursive destruction | |
if(pointee_) delete pointee_; | |
pointee_ = src.pointee_; | |
}else{ | |
pointee_ = src.pointee_; | |
//src.pointee_ = 0; // NOT DESTRUCTIVE | |
counter = src.counter; | |
counter->increment(); | |
} | |
} | |
return *this; | |
} |
If you call delete pointee_ before incrementing the counter of src, it triggers recursive destruction in the above case.
For a large graph cached in memory, it's good to have some kind of protection or grouping methods.