diff options
author | Mathias Stearn <mathias@10gen.com> | 2019-01-25 17:56:37 -0500 |
---|---|---|
committer | Mathias Stearn <mathias@10gen.com> | 2019-02-07 14:06:10 -0500 |
commit | 24a0276f123949081c1cd0d1c7876551b3f065a1 (patch) | |
tree | a30e1428cd7a78e8177515ca87cfdc4617cd01a0 /src/mongo/db | |
parent | 2f1881db90c1ae016561fa15274a5b5dee46e986 (diff) | |
download | mongo-24a0276f123949081c1cd0d1c7876551b3f065a1.tar.gz |
SERVER-39209 DocumentStorage::clone() shouldn't allocate a buffer is source didn't have one
Diffstat (limited to 'src/mongo/db')
-rw-r--r-- | src/mongo/db/pipeline/document.cpp | 50 | ||||
-rw-r--r-- | src/mongo/db/pipeline/document_value_test.cpp | 8 |
2 files changed, 38 insertions, 20 deletions
diff --git a/src/mongo/db/pipeline/document.cpp b/src/mongo/db/pipeline/document.cpp index b6274da457a..61bdf65c0be 100644 --- a/src/mongo/db/pipeline/document.cpp +++ b/src/mongo/db/pipeline/document.cpp @@ -193,29 +193,39 @@ void DocumentStorage::reserveFields(size_t expectedFields) { intrusive_ptr<DocumentStorage> DocumentStorage::clone() const { auto out = make_intrusive<DocumentStorage>(); - // Make a copy of the buffer. - // It is very important that the positions of each field are the same after cloning. - const size_t bufferBytes = allocatedBytes(); - out->_buffer = new char[bufferBytes]; - out->_bufferEnd = out->_buffer + (_bufferEnd - _buffer); - if (bufferBytes > 0) { + if (_buffer) { + // Make a copy of the buffer with the fields. + // It is very important that the positions of each field are the same after cloning. + const size_t bufferBytes = allocatedBytes(); + out->_buffer = new char[bufferBytes]; + out->_bufferEnd = out->_buffer + (_bufferEnd - _buffer); memcpy(out->_buffer, _buffer, bufferBytes); + + out->_hashTabMask = _hashTabMask; + out->_usedBytes = _usedBytes; + out->_numFields = _numFields; + + dassert(out->allocatedBytes() == bufferBytes); + + // Tell values that they have been memcpyed (updates ref counts) + for (DocumentStorageIterator it = out->iteratorAll(); !it.atEnd(); it.advance()) { + it->val.memcpyed(); + } + } else { + // If we don't have a buffer, these fields should still be in their initial state. + dassert(out->_hashTabMask == _hashTabMask); + dassert(out->_usedBytes == _usedBytes); + dassert(out->_numFields == _numFields); } - // Copy remaining fields - out->_usedBytes = _usedBytes; - out->_numFields = _numFields; - out->_hashTabMask = _hashTabMask; - out->_metaFields = _metaFields; - out->_textScore = _textScore; - out->_randVal = _randVal; - out->_sortKey = _sortKey.getOwned(); - out->_geoNearDistance = _geoNearDistance; - out->_geoNearPoint = _geoNearPoint.getOwned(); - - // Tell values that they have been memcpyed (updates ref counts) - for (DocumentStorageIterator it = out->iteratorAll(); !it.atEnd(); it.advance()) { - it->val.memcpyed(); + // Copy metadata + if (_metaFields.any()) { + out->_metaFields = _metaFields; + out->_textScore = _textScore; + out->_randVal = _randVal; + out->_sortKey = _sortKey.getOwned(); + out->_geoNearDistance = _geoNearDistance; + out->_geoNearPoint = _geoNearPoint.getOwned(); } return out; diff --git a/src/mongo/db/pipeline/document_value_test.cpp b/src/mongo/db/pipeline/document_value_test.cpp index 82f264cdf60..7d9c82bfa5d 100644 --- a/src/mongo/db/pipeline/document_value_test.cpp +++ b/src/mongo/db/pipeline/document_value_test.cpp @@ -111,6 +111,14 @@ TEST(DocumentConstruction, FromEmptyDocumentClone) { // Prior to SERVER-26462, cloning an empty document would cause a segmentation fault. Document documentClone = document.clone(); ASSERT_DOCUMENT_EQ(document, documentClone); + + // Prior to SERVER-39209 this would make ASAN complain. + Document documentClone2 = documentClone.clone(); + ASSERT_DOCUMENT_EQ(document, documentClone2); + + // For good measure, try a third clone + Document documentClone3 = documentClone2.clone(); + ASSERT_DOCUMENT_EQ(document, documentClone3); } /** |