shared_ptr<>::reset(): fix implementation which is not thread safe (#378

)
electronicarts · Jul 7, 2020 · fc6af13 · fc6af13
1 parent e1b7ab8
commit fc6af13
Show file tree

Hide file tree

Showing 2 changed files with 105 additions and 7 deletions.
diff --git a/include/EASTL/shared_ptr.h b/include/EASTL/shared_ptr.h
@@ -162,15 +162,11 @@ namespace eastl
 	inline void ref_count_sp::release()
 	{
 		EASTL_ASSERT((mRefCount > 0) && (mWeakRefCount > 0));
-		if(Internal::atomic_decrement(&mRefCount) > 0)
-			Internal::atomic_decrement(&mWeakRefCount);
-		else
-		{
+		if(Internal::atomic_decrement(&mRefCount) == 0)
 			free_value();
 
-			if(Internal::atomic_decrement(&mWeakRefCount) == 0)
-				free_ref_count_sp();
-		}
+		if(Internal::atomic_decrement(&mWeakRefCount) == 0)
+			free_ref_count_sp();
 	}
 
 	inline void ref_count_sp::weak_addref() EA_NOEXCEPT

diff --git a/test/source/TestSmartPtr.cpp b/test/source/TestSmartPtr.cpp
@@ -21,6 +21,7 @@
 #include <EASTL/unique_ptr.h>
 #include <EASTL/weak_ptr.h>
 #include <eathread/eathread_thread.h>
+#include <future>
 
 EA_DISABLE_ALL_VC_WARNINGS()
 #include <stdio.h>
@@ -1392,6 +1393,29 @@ static int Test_shared_ptr()
 #endif
 
 
+template<typename Fct1, typename Fct2>
+void Try_to_invoke_2tasks_at_same_time(const Fct1& fct1, const Fct2& fct2)
+{
+	std::atomic_uint32_t waitThread = true;
+	std::atomic_uint32_t waitMainThread = true;
+
+	std::future<void> f1 = std::async(
+		[&]
+		{
+			waitThread = false;
+			while(waitMainThread)
+			{
+			}
+			fct1();
+		});
+
+	waitMainThread = false;
+	while(waitThread)
+	{
+	}
+	fct2();
+}
+
 static int Test_shared_ptr_thread()
 {
 	using namespace SmartPtrTest;
@@ -1486,6 +1510,84 @@ static int Test_shared_ptr_thread()
 	EATEST_VERIFY(A::mCount == 0);
 	TestObject::Reset();
 
+	{
+		// Check that counter inside shared_ptr<> is thread safe when using reset().
+		for(uint32_t counter = 0; counter < 200000; ++counter)
+		{
+			eastl::shared_ptr<double> valueSPtr1(new double(0.));
+			eastl::shared_ptr<double> valueSPtr2(valueSPtr1);
+
+			Try_to_invoke_2tasks_at_same_time(
+				[&]
+				{
+					valueSPtr1.reset();
+				},
+				[&]
+				{
+					valueSPtr2.reset();
+				});
+		}
+	}
+
+	{
+		// Check that counter inside shared_ptr<> and weak_ptr<> is thread safe when using reset().
+		for(uint32_t counter = 0; counter < 200000; ++counter)
+		{
+			eastl::shared_ptr<double> valueSPtr(new double(0.));
+			eastl::weak_ptr<double> valueWPtr(valueSPtr);
+
+			Try_to_invoke_2tasks_at_same_time(
+				[&]
+				{
+					valueSPtr.reset();
+				},
+				[&]
+				{
+					valueWPtr.reset();
+				});
+		}
+	}
+
+	{
+		// Check that counter inside shared_ptr<> is thread safe when using operator =().
+		for(uint32_t counter = 0; counter < 200000; ++counter)
+		{
+			eastl::shared_ptr<double> valueSPtr(new double(0.));
+			eastl::weak_ptr<double> valueWPtr(valueSPtr);
+			eastl::shared_ptr<double> otherValueSPtr(new double(0.));
+
+			Try_to_invoke_2tasks_at_same_time(
+				[&]
+				{
+					valueSPtr = otherValueSPtr;
+				},
+				[&]
+				{
+					valueWPtr = otherValueSPtr;
+				});
+		}
+	}
+
+	{
+		// Check that counter inside shared_ptr<> and weak_ptr<> is thread safe when using operator =().
+		for(uint32_t counter = 0; counter < 200000; ++counter)
+		{
+			eastl::shared_ptr<double> valueSPtr(new double(0.));
+			eastl::shared_ptr<double> valueWPtr(valueSPtr);
+			eastl::shared_ptr<double> otherValueSPtr(new double(0.));
+
+			Try_to_invoke_2tasks_at_same_time(
+				[&]
+				{
+					valueSPtr = otherValueSPtr;
+				},
+				[&]
+				{
+					valueWPtr = otherValueSPtr;
+				});
+		}
+	}
+
 	return nErrorCount;
 }