From a4d6589ff26007c966db8912e4dae1aa937a5968 Mon Sep 17 00:00:00 2001 From: Jay Berkenbilt Date: Thu, 25 Feb 2021 06:34:03 -0500 Subject: Have QPDFObjectHandle notice when replaceObject was called This results in a performance penalty of 1% to 2% when replaceObject and swapObjects are never called and a somewhat larger penalty if they are called, but it's worth it to avoid very confusing behavior as discussed in depth in qpdf#507. --- libqpdf/QPDF.cc | 27 +++++++++++++++++++++++++++ libqpdf/QPDFObjectHandle.cc | 6 ++++++ 2 files changed, 33 insertions(+) (limited to 'libqpdf') diff --git a/libqpdf/QPDF.cc b/libqpdf/QPDF.cc index 1935d420..b751e602 100644 --- a/libqpdf/QPDF.cc +++ b/libqpdf/QPDF.cc @@ -210,6 +210,7 @@ QPDF::Members::Members() : immediate_copy_from(false), in_parse(false), parsed(false), + ever_replaced_objects(false), first_xref_item_offset(0), uncompressed_after_compressed(false) { @@ -2063,6 +2064,30 @@ QPDF::readObjectAtOffset(bool try_recovery, return oh; } +bool +QPDF::objectChanged(QPDFObjGen const& og, PointerHolder& oph) +{ + // See if the object cached at og, if any, is the one passed in. + // QPDFObjectHandle uses this to detect outdated handles to + // replaced or swapped objects. This is a somewhat expensive check + // because it happens with every dereference of a + // QPDFObjectHandle. To reduce the hit somewhat, short-circuit the + // check if we never called a function that replaces an object + // already in cache. It is important for functions that do this to + // set ever_replaced_objects = true. + + if (! this->m->ever_replaced_objects) + { + return false; + } + auto c = this->m->obj_cache.find(og); + if (c == this->m->obj_cache.end()) + { + return true; + } + return (c->second.object.getPointer() != oph.getPointer()); +} + PointerHolder QPDF::resolve(int objid, int generation) { @@ -2308,6 +2333,7 @@ QPDF::replaceObject(int objid, int generation, QPDFObjectHandle oh) // Replace the object in the object cache QPDFObjGen og(objid, generation); + this->m->ever_replaced_objects = true; this->m->obj_cache[og] = ObjCache(QPDFObjectHandle::ObjAccessor::getObject(oh), -1, -1); } @@ -2695,6 +2721,7 @@ QPDF::swapObjects(int objid1, int generation1, int objid2, int generation2) QPDFObjGen og1(objid1, generation1); QPDFObjGen og2(objid2, generation2); ObjCache t = this->m->obj_cache[og1]; + this->m->ever_replaced_objects = true; this->m->obj_cache[og1] = this->m->obj_cache[og2]; this->m->obj_cache[og2] = t; } diff --git a/libqpdf/QPDFObjectHandle.cc b/libqpdf/QPDFObjectHandle.cc index c650bdea..6d4f10ce 100644 --- a/libqpdf/QPDFObjectHandle.cc +++ b/libqpdf/QPDFObjectHandle.cc @@ -3194,6 +3194,12 @@ QPDFObjectHandle::dereference() throw std::logic_error( "attempted to dereference an uninitialized QPDFObjectHandle"); } + if (this->obj.getPointer() && this->objid && + QPDF::Resolver::objectChanged( + this->qpdf, QPDFObjGen(this->objid, this->generation), this->obj)) + { + this->obj = nullptr; + } if (this->obj.getPointer() == 0) { PointerHolder obj = QPDF::Resolver::resolve( -- cgit v1.2.3-54-g00ecf