summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
Diffstat
-rw-r--r--src/mongo/db/pipeline/document.cpp2
-rw-r--r--src/mongo/db/pipeline/document.h7
-rw-r--r--src/mongo/db/pipeline/expression.h4
-rw-r--r--src/mongo/db/pipeline/value.cpp22
-rw-r--r--src/mongo/db/pipeline/value.h3
-rw-r--r--src/mongo/db/pipeline/value_internal.h17
-rw-r--r--src/mongo/util/future.h54
-rw-r--r--src/mongo/util/intrusive_counter.h36
8 files changed, 59 insertions, 86 deletions
diff --git a/src/mongo/db/pipeline/document.cpp b/src/mongo/db/pipeline/document.cpp
index f871b33be9a..b6274da457a 100644
--- a/src/mongo/db/pipeline/document.cpp
+++ b/src/mongo/db/pipeline/document.cpp
@@ -191,7 +191,7 @@ void DocumentStorage::reserveFields(size_t expectedFields) {
}
intrusive_ptr<DocumentStorage> DocumentStorage::clone() const {
- intrusive_ptr<DocumentStorage> out(new DocumentStorage());
+ 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.
diff --git a/src/mongo/db/pipeline/document.h b/src/mongo/db/pipeline/document.h
index 7a969d59321..822ec26c886 100644
--- a/src/mongo/db/pipeline/document.h
+++ b/src/mongo/db/pipeline/document.h
@@ -306,7 +306,8 @@ private:
friend class MutableDocument;
friend class MutableValue;
- explicit Document(const DocumentStorage* ptr) : _storage(ptr){};
+ explicit Document(boost::intrusive_ptr<const DocumentStorage>&& ptr)
+ : _storage(std::move(ptr)) {}
const DocumentStorage& storage() const {
return (_storage ? *_storage : DocumentStorage::emptyDoc());
@@ -391,7 +392,7 @@ private:
// result in an allocation where none is needed, in practice this is only called
// when we are about to add a field to the sub-document so this just changes where
// the allocation is done.
- _val = Value(Document(new DocumentStorage()));
+ _val = Value(Document(make_intrusive<DocumentStorage>()));
}
return _val._storage.genericRCPtr;
@@ -598,7 +599,7 @@ private:
return const_cast<DocumentStorage&>(*storagePtr());
}
DocumentStorage& newStorage() {
- reset(new DocumentStorage);
+ reset(make_intrusive<DocumentStorage>());
return const_cast<DocumentStorage&>(*storagePtr());
}
DocumentStorage& clonedStorage() {
diff --git a/src/mongo/db/pipeline/expression.h b/src/mongo/db/pipeline/expression.h
index 31d1e185d34..d02df48f9d7 100644
--- a/src/mongo/db/pipeline/expression.h
+++ b/src/mongo/db/pipeline/expression.h
@@ -312,11 +312,11 @@ public:
const boost::intrusive_ptr<ExpressionContext>& expCtx,
BSONElement bsonExpr,
const VariablesParseState& vps) {
- boost::intrusive_ptr<ExpressionNaryBase> expr = new SubClass(expCtx);
+ auto expr = make_intrusive<SubClass>(expCtx);
ExpressionVector args = parseArguments(expCtx, bsonExpr, vps);
expr->validateArguments(args);
expr->vpOperand = args;
- return expr;
+ return std::move(expr);
}
protected:
diff --git a/src/mongo/db/pipeline/value.cpp b/src/mongo/db/pipeline/value.cpp
index 4357c7d15ea..ab3d6a77af4 100644
--- a/src/mongo/db/pipeline/value.cpp
+++ b/src/mongo/db/pipeline/value.cpp
@@ -125,9 +125,9 @@ void ValueStorage::putDocument(const Document& d) {
putRefCountable(d._storage);
}
-void ValueStorage::putVector(const RCVector* vec) {
- fassert(16485, vec);
- putRefCountable(vec);
+void ValueStorage::putVector(boost::intrusive_ptr<RCVector>&& vec) {
+ fassert(16485, bool(vec));
+ putRefCountable(std::move(vec));
}
void ValueStorage::putRegEx(const BSONRegEx& re) {
@@ -180,11 +180,11 @@ Value::Value(const BSONElement& elem) : _storage(elem.type()) {
}
case Array: {
- intrusive_ptr<RCVector> vec(new RCVector);
+ auto vec = make_intrusive<RCVector>();
BSONForEach(sub, elem.embeddedObject()) {
vec->vec.push_back(Value(sub));
}
- _storage.putVector(vec.get());
+ _storage.putVector(std::move(vec));
break;
}
@@ -242,29 +242,29 @@ Value::Value(const BSONElement& elem) : _storage(elem.type()) {
}
Value::Value(const BSONArray& arr) : _storage(Array) {
- intrusive_ptr<RCVector> vec(new RCVector);
+ auto vec = make_intrusive<RCVector>();
BSONForEach(sub, arr) {
vec->vec.push_back(Value(sub));
}
- _storage.putVector(vec.get());
+ _storage.putVector(std::move(vec));
}
Value::Value(const vector<BSONObj>& vec) : _storage(Array) {
- intrusive_ptr<RCVector> storageVec(new RCVector);
+ auto storageVec = make_intrusive<RCVector>();
storageVec->vec.reserve(vec.size());
for (auto&& obj : vec) {
storageVec->vec.push_back(Value(obj));
}
- _storage.putVector(storageVec.get());
+ _storage.putVector(std::move(storageVec));
}
Value::Value(const vector<Document>& vec) : _storage(Array) {
- intrusive_ptr<RCVector> storageVec(new RCVector);
+ auto storageVec = make_intrusive<RCVector>();
storageVec->vec.reserve(vec.size());
for (auto&& obj : vec) {
storageVec->vec.push_back(Value(obj));
}
- _storage.putVector(storageVec.get());
+ _storage.putVector(std::move(storageVec));
}
Value Value::createIntOrLong(long long longValue) {
diff --git a/src/mongo/db/pipeline/value.h b/src/mongo/db/pipeline/value.h
index e6db7e8f0c7..ec105986ab8 100644
--- a/src/mongo/db/pipeline/value.h
+++ b/src/mongo/db/pipeline/value.h
@@ -107,7 +107,8 @@ public:
explicit Value(const BSONArray& arr);
explicit Value(const std::vector<BSONObj>& vec);
explicit Value(const std::vector<Document>& vec);
- explicit Value(std::vector<Value> vec) : _storage(Array, new RCVector(std::move(vec))) {}
+ explicit Value(std::vector<Value> vec)
+ : _storage(Array, make_intrusive<RCVector>(std::move(vec))) {}
explicit Value(const BSONBinData& bd) : _storage(BinData, bd) {}
explicit Value(const BSONRegEx& re) : _storage(RegEx, re) {}
explicit Value(const BSONCodeWScope& cws) : _storage(CodeWScope, cws) {}
diff --git a/src/mongo/db/pipeline/value_internal.h b/src/mongo/db/pipeline/value_internal.h
index b6fbee372c3..810a1e4b2c7 100644
--- a/src/mongo/db/pipeline/value_internal.h
+++ b/src/mongo/db/pipeline/value_internal.h
@@ -124,10 +124,10 @@ public:
type = t;
putDocument(d);
}
- ValueStorage(BSONType t, const RCVector* a) {
+ ValueStorage(BSONType t, boost::intrusive_ptr<RCVector>&& a) {
zero();
type = t;
- putVector(a);
+ putVector(std::move(a));
}
ValueStorage(BSONType t, StringData s) {
zero();
@@ -221,7 +221,7 @@ public:
/// These are only to be called during Value construction on an empty Value
void putString(StringData s);
- void putVector(const RCVector* v);
+ void putVector(boost::intrusive_ptr<RCVector>&& v);
void putDocument(const Document& d);
void putRegEx(const BSONRegEx& re);
void putBinData(const BSONBinData& bd) {
@@ -230,22 +230,21 @@ public:
}
void putDBRef(const BSONDBRef& dbref) {
- putRefCountable(new RCDBRef(dbref.ns.toString(), dbref.oid));
+ putRefCountable(make_intrusive<RCDBRef>(dbref.ns.toString(), dbref.oid));
}
void putCodeWScope(const BSONCodeWScope& cws) {
- putRefCountable(new RCCodeWScope(cws.code.toString(), cws.scope));
+ putRefCountable(make_intrusive<RCCodeWScope>(cws.code.toString(), cws.scope));
}
void putDecimal(const Decimal128& d) {
- putRefCountable(new RCDecimal(d));
+ putRefCountable(make_intrusive<RCDecimal>(d));
}
- void putRefCountable(boost::intrusive_ptr<const RefCountable> ptr) {
- genericRCPtr = ptr.get();
+ void putRefCountable(boost::intrusive_ptr<const RefCountable>&& ptr) {
+ genericRCPtr = ptr.detach();
if (genericRCPtr) {
- intrusive_ptr_add_ref(genericRCPtr);
refCounter = true;
}
DEV verifyRefCountingIfShould();
diff --git a/src/mongo/util/future.h b/src/mongo/util/future.h
index f8edd5ce81d..3033bec5687 100644
--- a/src/mongo/util/future.h
+++ b/src/mongo/util/future.h
@@ -253,58 +253,6 @@ struct FutureContinuationResultImpl<Status> {
using type = void;
};
-/**
- * A base class that handles the ref-count for boost::intrusive_ptr compatibility.
- *
- * This is taken from RefCountable which is used for the aggregation types, adding in a way to set
- * the refcount non-atomically during initialization. Also using explicit memory orderings for all
- * operations on the count.
- * TODO look into merging back.
- */
-class FutureRefCountable {
- MONGO_DISALLOW_COPYING(FutureRefCountable);
-
-public:
- /**
- * Sets the refcount to count, assuming it is currently one less. This should only be used
- * during logical initialization before another thread could possibly have access to this
- * object.
- */
- void threadUnsafeIncRefCountTo(uint32_t count) const {
- dassert(_count.load(std::memory_order_relaxed) == (count - 1));
- _count.store(count, std::memory_order_relaxed);
- }
-
- friend void intrusive_ptr_add_ref(const FutureRefCountable* ptr) {
- // See this for a description of why relaxed is OK here. It is also used in libc++.
- // http://www.boost.org/doc/libs/1_66_0/doc/html/atomic/usage_examples.html#boost_atomic.usage_examples.example_reference_counters.discussion
- ptr->_count.fetch_add(1, std::memory_order_relaxed);
- };
-
- friend void intrusive_ptr_release(const FutureRefCountable* ptr) {
- if (ptr->_count.fetch_sub(1, std::memory_order_acq_rel) == 1) {
- delete ptr;
- }
- };
-
-protected:
- FutureRefCountable() = default;
- virtual ~FutureRefCountable() = default;
-
-private:
- mutable std::atomic<uint32_t> _count{0}; // NOLINT
-};
-
-template <typename T,
- typename... Args,
- typename = std::enable_if_t<std::is_base_of<FutureRefCountable, T>::value>>
-boost::intrusive_ptr<T> make_intrusive(Args&&... args) {
- auto ptr = new T(std::forward<Args>(args)...);
- ptr->threadUnsafeIncRefCountTo(1);
- return boost::intrusive_ptr<T>(ptr, /*add ref*/ false);
-}
-
-
template <typename T>
struct SharedStateImpl;
@@ -350,7 +298,7 @@ enum class SSBState : uint8_t {
kFinished,
};
-class SharedStateBase : public FutureRefCountable {
+class SharedStateBase : public RefCountable {
public:
SharedStateBase(const SharedStateBase&) = delete;
SharedStateBase(SharedStateBase&&) = delete;
diff --git a/src/mongo/util/intrusive_counter.h b/src/mongo/util/intrusive_counter.h
index b78f83809df..515e5554591 100644
--- a/src/mongo/util/intrusive_counter.h
+++ b/src/mongo/util/intrusive_counter.h
@@ -47,17 +47,32 @@ class RefCountable {
public:
/// If false you have exclusive access to this object. This is useful for implementing COW.
bool isShared() const {
- // TODO: switch to unfenced read method after SERVER-6973
- return reinterpret_cast<unsigned&>(_count) > 1;
+ // This needs to be at least acquire to ensure that it is sequenced-after any
+ // intrusive_ptr_release calls. Otherwise there is a subtle race where the releaser's memory
+ // accesses could see writes done by a thread that thinks it has exclusive access to this
+ // object. Note that acquire reads are free on x86 and cheap on most other platforms.
+ return _count.load(std::memory_order_acquire) > 1;
+ }
+
+ /**
+ * Sets the refcount to count, assuming it is currently one less. This should only be used
+ * during logical initialization before another thread could possibly have access to this
+ * object.
+ */
+ void threadUnsafeIncRefCountTo(uint32_t count) const {
+ dassert(_count.load(std::memory_order_relaxed) == (count - 1));
+ _count.store(count, std::memory_order_relaxed);
}
friend void intrusive_ptr_add_ref(const RefCountable* ptr) {
- ptr->_count.addAndFetch(1);
+ // See this for a description of why relaxed is OK here. It is also used in libc++.
+ // http://www.boost.org/doc/libs/1_66_0/doc/html/atomic/usage_examples.html#boost_atomic.usage_examples.example_reference_counters.discussion
+ ptr->_count.fetch_add(1, std::memory_order_relaxed);
};
friend void intrusive_ptr_release(const RefCountable* ptr) {
- if (ptr->_count.subtractAndFetch(1) == 0) {
- delete ptr; // uses subclass destructor and operator delete
+ if (ptr->_count.fetch_sub(1, std::memory_order_acq_rel) == 1) {
+ delete ptr;
}
};
@@ -66,9 +81,18 @@ protected:
virtual ~RefCountable() {}
private:
- mutable AtomicWord<unsigned> _count; // default initialized to 0
+ mutable std::atomic<uint32_t> _count{0}; // NOLINT
};
+template <typename T,
+ typename... Args,
+ typename = std::enable_if_t<std::is_base_of<RefCountable, T>::value>>
+boost::intrusive_ptr<T> make_intrusive(Args&&... args) {
+ auto ptr = new T(std::forward<Args>(args)...);
+ ptr->threadUnsafeIncRefCountTo(1);
+ return boost::intrusive_ptr<T>(ptr, /*add ref*/ false);
+}
+
/// This is an immutable reference-counted string
class RCString : public RefCountable {
public:
View Lorry Controller Status