putil: keep reference to objects queued for writing

This prevents a crash when a reference counted object is destroyed right after a write_pointer call.

Fixes #310
This commit is contained in:
rdb 2018-05-05 23:10:41 +02:00
parent 8a98bf42a3
commit 549301d0f0
2 changed files with 30 additions and 3 deletions

View File

@ -94,6 +94,10 @@ BamWriter::
for (si = _state_map.begin(); si != _state_map.end(); ++si) { for (si = _state_map.begin(); si != _state_map.end(); ++si) {
TypedWritable *object = (TypedWritable *)(*si).first; TypedWritable *object = (TypedWritable *)(*si).first;
object->remove_bam_writer(this); object->remove_bam_writer(this);
if ((*si).second._refcount != nullptr) {
unref_delete((*si).second._refcount);
}
} }
} }
@ -529,6 +533,9 @@ object_destructs(TypedWritable *object) {
// we're in trouble when we do write it out. // we're in trouble when we do write it out.
nassertv(!(*si).second._written_seq.is_initial()); nassertv(!(*si).second._written_seq.is_initial());
// This cannot be called if we are still holding a reference to it.
nassertv((*si).second._refcount == nullptr);
int object_id = (*si).second._object_id; int object_id = (*si).second._object_id;
_freed_object_ids.push_back(object_id); _freed_object_ids.push_back(object_id);
@ -606,8 +613,10 @@ enqueue_object(const TypedWritable *object) {
// No, it hasn't, so assign it the next number in sequence arbitrarily. // No, it hasn't, so assign it the next number in sequence arbitrarily.
object_id = _next_object_id; object_id = _next_object_id;
bool inserted = StateMap::iterator si;
_state_map.insert(StateMap::value_type(object, StoreState(_next_object_id))).second; bool inserted;
tie(si, inserted) =
_state_map.insert(StateMap::value_type(object, StoreState(_next_object_id)));
nassertr(inserted, false); nassertr(inserted, false);
// Store ourselves on the TypedWritable so that we get notified when it // Store ourselves on the TypedWritable so that we get notified when it
@ -615,6 +624,14 @@ enqueue_object(const TypedWritable *object) {
(const_cast<TypedWritable*>(object))->add_bam_writer(this); (const_cast<TypedWritable*>(object))->add_bam_writer(this);
_next_object_id++; _next_object_id++;
// Increase the reference count if this inherits from ReferenceCount,
// until we get a chance to write this object for the first time.
const ReferenceCount *rc = ((TypedWritable *)object)->as_reference_count();
if (rc != nullptr) {
rc->ref();
(*si).second._refcount = rc;
}
} else { } else {
// Yes, it has; get the object ID. // Yes, it has; get the object ID.
object_id = (*si).second._object_id; object_id = (*si).second._object_id;
@ -703,6 +720,15 @@ flush_queue() {
(*si).second._written_seq = _writing_seq; (*si).second._written_seq = _writing_seq;
(*si).second._modified = object->get_bam_modified(); (*si).second._modified = object->get_bam_modified();
// Now release any reference we hold to it, so that it may destruct.
const ReferenceCount *rc = (*si).second._refcount;
if (rc != nullptr) {
// We need to assign this pointer to null before deleting the object,
// since that may end up calling object_destructs.
(*si).second._refcount = nullptr;
unref_delete(rc);
}
} else { } else {
// On subsequent times when we write a particular object, we write // On subsequent times when we write a particular object, we write
// simply TypeHandle::none(), followed by the object ID. The occurrence // simply TypeHandle::none(), followed by the object ID. The occurrence

View File

@ -140,8 +140,9 @@ private:
int _object_id; int _object_id;
UpdateSeq _written_seq; UpdateSeq _written_seq;
UpdateSeq _modified; UpdateSeq _modified;
const ReferenceCount *_refcount;
StoreState(int object_id) : _object_id(object_id) {} StoreState(int object_id) : _object_id(object_id), _refcount(nullptr) {}
}; };
typedef phash_map<const TypedWritable *, StoreState, pointer_hash> StateMap; typedef phash_map<const TypedWritable *, StoreState, pointer_hash> StateMap;
StateMap _state_map; StateMap _state_map;