Commit 9c86ba40d80ee3b58801dfe77e47fbc5a9dd6066
1 parent
6d2db68f
Fix commit 805c1ad : Reset QPDFValue::qpdf and QPDFValue::og when ...
On destruction of the QPDF object replace all indirect object references with direct nulls. Remove all existing code to release resolved references. Fixes performance issue due to interaction of resetting QPDFValue::qpdf and og members and prior code.
Showing
13 changed files
with
6 additions
and
90 deletions
include/qpdf/QPDFObjectHandle.hh
| @@ -1500,23 +1500,6 @@ class QPDFObjectHandle | @@ -1500,23 +1500,6 @@ class QPDFObjectHandle | ||
| 1500 | }; | 1500 | }; |
| 1501 | friend class ObjAccessor; | 1501 | friend class ObjAccessor; |
| 1502 | 1502 | ||
| 1503 | - // Provide access to specific classes for recursive | ||
| 1504 | - // releaseResolved(). | ||
| 1505 | - class ReleaseResolver | ||
| 1506 | - { | ||
| 1507 | - friend class QPDF_Dictionary; | ||
| 1508 | - friend class QPDF_Stream; | ||
| 1509 | - friend class SparseOHArray; | ||
| 1510 | - | ||
| 1511 | - private: | ||
| 1512 | - static void | ||
| 1513 | - releaseResolved(QPDFObjectHandle& o) | ||
| 1514 | - { | ||
| 1515 | - o.releaseResolved(); | ||
| 1516 | - } | ||
| 1517 | - }; | ||
| 1518 | - friend class ReleaseResolver; | ||
| 1519 | - | ||
| 1520 | // Convenience routine: Throws if the assumption is violated. Your | 1503 | // Convenience routine: Throws if the assumption is violated. Your |
| 1521 | // code will be better if you call one of the isType methods and | 1504 | // code will be better if you call one of the isType methods and |
| 1522 | // handle the case of the type being wrong, but these can be | 1505 | // handle the case of the type being wrong, but these can be |
| @@ -1614,8 +1597,6 @@ class QPDFObjectHandle | @@ -1614,8 +1597,6 @@ class QPDFObjectHandle | ||
| 1614 | bool first_level_only, | 1597 | bool first_level_only, |
| 1615 | bool stop_at_streams); | 1598 | bool stop_at_streams); |
| 1616 | void shallowCopyInternal(QPDFObjectHandle& oh, bool first_level_only); | 1599 | void shallowCopyInternal(QPDFObjectHandle& oh, bool first_level_only); |
| 1617 | - void releaseResolved(); | ||
| 1618 | - | ||
| 1619 | void setParsedOffset(qpdf_offset_t offset); | 1600 | void setParsedOffset(qpdf_offset_t offset); |
| 1620 | void parseContentStream_internal( | 1601 | void parseContentStream_internal( |
| 1621 | std::string const& description, ParserCallbacks* callbacks); | 1602 | std::string const& description, ParserCallbacks* callbacks); |
libqpdf/QPDF.cc
| @@ -249,17 +249,16 @@ QPDF::~QPDF() | @@ -249,17 +249,16 @@ QPDF::~QPDF() | ||
| 249 | // std::shared_ptr objects will prevent the objects from being | 249 | // std::shared_ptr objects will prevent the objects from being |
| 250 | // deleted. Walk through all objects in the object cache, which | 250 | // deleted. Walk through all objects in the object cache, which |
| 251 | // is those objects that we read from the file, and break all | 251 | // is those objects that we read from the file, and break all |
| 252 | - // resolved references. At this point, obviously no one is still | 252 | + // resolved indirect references by replacing them with direct |
| 253 | + // null objects. At this point, obviously no one is still | ||
| 253 | // using the QPDF object, but we'll explicitly clear the xref | 254 | // using the QPDF object, but we'll explicitly clear the xref |
| 254 | // table anyway just to prevent any possibility of resolve() | 255 | // table anyway just to prevent any possibility of resolve() |
| 255 | // succeeding. Note that we can't break references like this at | 256 | // succeeding. Note that we can't break references like this at |
| 256 | - // any time when the QPDF object is active. If we do, the next | ||
| 257 | - // reference will reread the object from the file, which would | ||
| 258 | - // have the effect of undoing any modifications that may have been | ||
| 259 | - // made to any of the objects. | 257 | + // any time when the QPDF object is active. |
| 260 | this->m->xref_table.clear(); | 258 | this->m->xref_table.clear(); |
| 259 | + auto null_obj = QPDF_Null::create(); | ||
| 261 | for (auto const& iter: this->m->obj_cache) { | 260 | for (auto const& iter: this->m->obj_cache) { |
| 262 | - iter.second.object->releaseResolved(); | 261 | + iter.second.object->assign(null_obj); |
| 263 | iter.second.object->resetObjGen(); | 262 | iter.second.object->resetObjGen(); |
| 264 | } | 263 | } |
| 265 | } | 264 | } |
libqpdf/QPDFObjectHandle.cc
| @@ -236,22 +236,6 @@ LastChar::getLastChar() | @@ -236,22 +236,6 @@ LastChar::getLastChar() | ||
| 236 | return this->last_char; | 236 | return this->last_char; |
| 237 | } | 237 | } |
| 238 | 238 | ||
| 239 | -void | ||
| 240 | -QPDFObjectHandle::releaseResolved() | ||
| 241 | -{ | ||
| 242 | - // Recursively break any resolved references to indirect objects. | ||
| 243 | - // Do not cross over indirect object boundaries to avoid an | ||
| 244 | - // infinite loop. This method may only be called during final | ||
| 245 | - // destruction. See comments in QPDF::~QPDF(). | ||
| 246 | - if (this->obj.get()) { | ||
| 247 | - if (isIndirect()) { | ||
| 248 | - this->obj = nullptr; | ||
| 249 | - } else { | ||
| 250 | - this->obj->releaseResolved(); | ||
| 251 | - } | ||
| 252 | - } | ||
| 253 | -} | ||
| 254 | - | ||
| 255 | qpdf_object_type_e | 239 | qpdf_object_type_e |
| 256 | QPDFObjectHandle::getTypeCode() | 240 | QPDFObjectHandle::getTypeCode() |
| 257 | { | 241 | { |
libqpdf/QPDF_Array.cc
| @@ -34,12 +34,6 @@ QPDF_Array::shallowCopy() | @@ -34,12 +34,6 @@ QPDF_Array::shallowCopy() | ||
| 34 | return create(elements); | 34 | return create(elements); |
| 35 | } | 35 | } |
| 36 | 36 | ||
| 37 | -void | ||
| 38 | -QPDF_Array::releaseResolved() | ||
| 39 | -{ | ||
| 40 | - this->elements.releaseResolved(); | ||
| 41 | -} | ||
| 42 | - | ||
| 43 | std::string | 37 | std::string |
| 44 | QPDF_Array::unparse() | 38 | QPDF_Array::unparse() |
| 45 | { | 39 | { |
libqpdf/QPDF_Dictionary.cc
| @@ -21,14 +21,6 @@ QPDF_Dictionary::shallowCopy() | @@ -21,14 +21,6 @@ QPDF_Dictionary::shallowCopy() | ||
| 21 | return create(items); | 21 | return create(items); |
| 22 | } | 22 | } |
| 23 | 23 | ||
| 24 | -void | ||
| 25 | -QPDF_Dictionary::releaseResolved() | ||
| 26 | -{ | ||
| 27 | - for (auto& iter: this->items) { | ||
| 28 | - QPDFObjectHandle::ReleaseResolver::releaseResolved(iter.second); | ||
| 29 | - } | ||
| 30 | -} | ||
| 31 | - | ||
| 32 | std::string | 24 | std::string |
| 33 | QPDF_Dictionary::unparse() | 25 | QPDF_Dictionary::unparse() |
| 34 | { | 26 | { |
libqpdf/QPDF_Stream.cc
| @@ -168,13 +168,6 @@ QPDF_Stream::getFilterOnWrite() const | @@ -168,13 +168,6 @@ QPDF_Stream::getFilterOnWrite() const | ||
| 168 | } | 168 | } |
| 169 | 169 | ||
| 170 | void | 170 | void |
| 171 | -QPDF_Stream::releaseResolved() | ||
| 172 | -{ | ||
| 173 | - this->stream_provider = nullptr; | ||
| 174 | - QPDFObjectHandle::ReleaseResolver::releaseResolved(this->stream_dict); | ||
| 175 | -} | ||
| 176 | - | ||
| 177 | -void | ||
| 178 | QPDF_Stream::setObjGen(QPDFObjGen const& og) | 171 | QPDF_Stream::setObjGen(QPDFObjGen const& og) |
| 179 | { | 172 | { |
| 180 | if (this->og.isIndirect()) { | 173 | if (this->og.isIndirect()) { |
libqpdf/SparseOHArray.cc
| @@ -49,14 +49,6 @@ SparseOHArray::remove_last() | @@ -49,14 +49,6 @@ SparseOHArray::remove_last() | ||
| 49 | } | 49 | } |
| 50 | 50 | ||
| 51 | void | 51 | void |
| 52 | -SparseOHArray::releaseResolved() | ||
| 53 | -{ | ||
| 54 | - for (auto& iter: this->elements) { | ||
| 55 | - QPDFObjectHandle::ReleaseResolver::releaseResolved(iter.second); | ||
| 56 | - } | ||
| 57 | -} | ||
| 58 | - | ||
| 59 | -void | ||
| 60 | SparseOHArray::setAt(size_t idx, QPDFObjectHandle oh) | 52 | SparseOHArray::setAt(size_t idx, QPDFObjectHandle oh) |
| 61 | { | 53 | { |
| 62 | if (idx >= this->n_elements) { | 54 | if (idx >= this->n_elements) { |
libqpdf/qpdf/QPDFObject.hh
| @@ -137,12 +137,6 @@ class QPDFObject | @@ -137,12 +137,6 @@ class QPDFObject | ||
| 137 | return dynamic_cast<T*>(value.get()); | 137 | return dynamic_cast<T*>(value.get()); |
| 138 | } | 138 | } |
| 139 | 139 | ||
| 140 | - void | ||
| 141 | - releaseResolved() | ||
| 142 | - { | ||
| 143 | - value->releaseResolved(); | ||
| 144 | - } | ||
| 145 | - | ||
| 146 | private: | 140 | private: |
| 147 | QPDFObject(QPDFObject const&) = delete; | 141 | QPDFObject(QPDFObject const&) = delete; |
| 148 | QPDFObject& operator=(QPDFObject const&) = delete; | 142 | QPDFObject& operator=(QPDFObject const&) = delete; |
libqpdf/qpdf/QPDFValue.hh
| @@ -86,10 +86,7 @@ class QPDFValue | @@ -86,10 +86,7 @@ class QPDFValue | ||
| 86 | og(og) | 86 | og(og) |
| 87 | { | 87 | { |
| 88 | } | 88 | } |
| 89 | - virtual void | ||
| 90 | - releaseResolved() | ||
| 91 | - { | ||
| 92 | - } | 89 | + |
| 93 | static std::shared_ptr<QPDFObject> do_create(QPDFValue*); | 90 | static std::shared_ptr<QPDFObject> do_create(QPDFValue*); |
| 94 | 91 | ||
| 95 | private: | 92 | private: |
libqpdf/qpdf/QPDF_Array.hh
| @@ -33,9 +33,6 @@ class QPDF_Array: public QPDFValue | @@ -33,9 +33,6 @@ class QPDF_Array: public QPDFValue | ||
| 33 | // API. Otherwise, these would be wrapped in accessor classes. | 33 | // API. Otherwise, these would be wrapped in accessor classes. |
| 34 | void addExplicitElementsToList(std::list<QPDFObjectHandle>&) const; | 34 | void addExplicitElementsToList(std::list<QPDFObjectHandle>&) const; |
| 35 | 35 | ||
| 36 | - protected: | ||
| 37 | - virtual void releaseResolved(); | ||
| 38 | - | ||
| 39 | private: | 36 | private: |
| 40 | QPDF_Array(std::vector<QPDFObjectHandle> const& items); | 37 | QPDF_Array(std::vector<QPDFObjectHandle> const& items); |
| 41 | QPDF_Array(SparseOHArray const& items); | 38 | QPDF_Array(SparseOHArray const& items); |
libqpdf/qpdf/QPDF_Dictionary.hh
| @@ -32,9 +32,6 @@ class QPDF_Dictionary: public QPDFValue | @@ -32,9 +32,6 @@ class QPDF_Dictionary: public QPDFValue | ||
| 32 | // Remove key, doing nothing if key does not exist | 32 | // Remove key, doing nothing if key does not exist |
| 33 | void removeKey(std::string const& key); | 33 | void removeKey(std::string const& key); |
| 34 | 34 | ||
| 35 | - protected: | ||
| 36 | - virtual void releaseResolved(); | ||
| 37 | - | ||
| 38 | private: | 35 | private: |
| 39 | QPDF_Dictionary(std::map<std::string, QPDFObjectHandle> const& items); | 36 | QPDF_Dictionary(std::map<std::string, QPDFObjectHandle> const& items); |
| 40 | std::map<std::string, QPDFObjectHandle> items; | 37 | std::map<std::string, QPDFObjectHandle> items; |
libqpdf/qpdf/QPDF_Stream.hh
| @@ -77,9 +77,6 @@ class QPDF_Stream: public QPDFValue | @@ -77,9 +77,6 @@ class QPDF_Stream: public QPDFValue | ||
| 77 | // when adding streams to files. | 77 | // when adding streams to files. |
| 78 | void setObjGen(QPDFObjGen const& og); | 78 | void setObjGen(QPDFObjGen const& og); |
| 79 | 79 | ||
| 80 | - protected: | ||
| 81 | - virtual void releaseResolved(); | ||
| 82 | - | ||
| 83 | private: | 80 | private: |
| 84 | QPDF_Stream( | 81 | QPDF_Stream( |
| 85 | QPDF*, | 82 | QPDF*, |
libqpdf/qpdf/SparseOHArray.hh
| @@ -12,7 +12,6 @@ class SparseOHArray | @@ -12,7 +12,6 @@ class SparseOHArray | ||
| 12 | void append(QPDFObjectHandle oh); | 12 | void append(QPDFObjectHandle oh); |
| 13 | QPDFObjectHandle at(size_t idx) const; | 13 | QPDFObjectHandle at(size_t idx) const; |
| 14 | void remove_last(); | 14 | void remove_last(); |
| 15 | - void releaseResolved(); | ||
| 16 | void setAt(size_t idx, QPDFObjectHandle oh); | 15 | void setAt(size_t idx, QPDFObjectHandle oh); |
| 17 | void erase(size_t idx); | 16 | void erase(size_t idx); |
| 18 | void insert(size_t idx, QPDFObjectHandle oh); | 17 | void insert(size_t idx, QPDFObjectHandle oh); |