From d69249188ff0a896079ccc9ff615bc896ad314f8 Mon Sep 17 00:00:00 2001 From: Billy Robert O'Neal III Date: Wed, 17 Oct 2018 17:52:49 -0700 Subject: [PATCH 1/5] Fix intermittent crashes in TestContinuationsWithTask. The one now marked 7 was creating a task that wrote to a stack variable, but never joined with that stack variable. This would cause random crashes whenever that stack position got clobbered. --- .../pplx/pplx_test/pplxtask_tests.cpp | 426 +++++++++--------- 1 file changed, 212 insertions(+), 214 deletions(-) diff --git a/Release/tests/functional/pplx/pplx_test/pplxtask_tests.cpp b/Release/tests/functional/pplx/pplx_test/pplxtask_tests.cpp index a1f6580edf..7f4f247f1b 100644 --- a/Release/tests/functional/pplx/pplx_test/pplxtask_tests.cpp +++ b/Release/tests/functional/pplx/pplx_test/pplxtask_tests.cpp @@ -203,7 +203,7 @@ TEST(TestTasks_default_construction) t1.wait(); LogFailure(L"t1.wait() should have thrown an exception"); } - catch (invalid_operation) + catch (invalid_operation) { } @@ -212,7 +212,7 @@ TEST(TestTasks_default_construction) t1.get(); LogFailure(L"t1.get() should have thrown an exception"); } - catch (invalid_operation) + catch (invalid_operation) { } @@ -224,7 +224,7 @@ TEST(TestTasks_default_construction) LogFailure(L"t1.then() should have thrown an exception"); } - catch (invalid_operation) + catch (invalid_operation) { } } @@ -268,7 +268,7 @@ TEST(TestTasks_void_tasks_default_construction) t1.wait(); LogFailure(L"t1.wait() should have thrown an exception"); } - catch (invalid_operation) + catch (invalid_operation) { } @@ -277,7 +277,7 @@ TEST(TestTasks_void_tasks_default_construction) t1.get(); LogFailure(L"t1.get() should have thrown an exception"); } - catch (invalid_operation) + catch (invalid_operation) { } @@ -286,7 +286,7 @@ TEST(TestTasks_void_tasks_default_construction) t1.then([] () { }); LogFailure(L"t1.contiue_with() should have thrown an exception"); } - catch (invalid_operation) + catch (invalid_operation) { } } @@ -332,7 +332,7 @@ TEST(TestTasks_constant_this) return 0; }); - auto func = [t1]() -> int + auto func = [t1]() -> int { t1.then([](int last) -> int { return last; @@ -344,7 +344,7 @@ TEST(TestTasks_constant_this) IsTrue(func() == 0, L"Tasks should be able to used inside a Lambda."); } #endif // _MSC_VER < 1700 -#endif // _MSC_VER +#endif // _MSC_VER } TEST(TestTasks_fire_and_forget) @@ -400,7 +400,7 @@ TEST(TestTaskCompletionEvents_basic2) task_completion_event tce; task completion(tce); auto completion2 = create_task(tce); - + task setEvent([=]() { tce.set(); }); @@ -496,7 +496,7 @@ TEST(TestTaskCompletionEvents_multiple_tasks) task t1(tce); task t2(tce); tce.set_exception(1); - + t1.then([=](task p) { try { @@ -639,7 +639,7 @@ TEST(TestTaskOperators_and_or2) IsTrue(vec[2] == 147, L"operator&& did not produce a correct vector[2]. Expected: 147, Actual: %d", vec[2]); IsTrue(vec[3] == 192, L"operator&& did not produce a correct vector[3]. Expected: 192, Actual: %d", vec[3]); int count = 0; - for(unsigned i = 0; i < vec.size(); i++) + for(unsigned i = 0; i < vec.size(); i++) count += vec[i]; return count; }); @@ -674,7 +674,7 @@ TEST(TestTaskOperators_and_or3) IsTrue(vec[2] == 147, L"operator&& did not produce a correct vector[2]. Expected: 147, Actual: %d", vec[2]); IsTrue(vec[3] == 192, L"operator&& did not produce a correct vector[3]. Expected: 192, Actual: %d", vec[3]); int count = 0; - for(unsigned i = 0; i < vec.size(); i++) + for(unsigned i = 0; i < vec.size(); i++) count += vec[i]; return count; }); @@ -862,7 +862,7 @@ TEST(TestTaskOperators_cancellation) auto t4 = (t1 && t2 && t3).then([=](std::vector vec) -> int { return vec[0] + vec[1] + vec[3]; }); - + ct.cancel(); tce.set(); @@ -885,7 +885,7 @@ TEST(TestTaskOperators_cancellation_and) task t3 ([]() -> void {}); auto t4 = (t1 && t2 && t3).then([=]() {}); - + ct.cancel(); tce.set(); @@ -918,7 +918,7 @@ TEST(TestTaskOperators_cancellation_or) auto t4 = (t1 || t2 || t3).then([=](int result) -> int { return result; }); - + ct1.cancel(); ct2.cancel(); ct3.cancel(); @@ -945,7 +945,7 @@ TEST(TestTaskOperators_cancellation_or2) task t3 = starter.then([]() -> void {}, ct3.get_token()); auto t4 = (t1 || t2 || t3).then([=]() {}); - + ct1.cancel(); ct2.cancel(); ct3.cancel(); @@ -1087,7 +1087,7 @@ TEST(TestTaskOperators_cancellation_exception) } task t3 = t1.then([&n](){ - pplx::details::atomic_add(n, 1L); // this should NOT execute, + pplx::details::atomic_add(n, 1L); // this should NOT execute, }); task t4 = t1.then([&n](task taskResult){ @@ -1381,249 +1381,247 @@ static int ThrowFunc() throw 42; } -TEST(TestContinuationsWithTask) +TEST(TestContinuationsWithTask1) { - { - int n2 = 0; + int n2 = 0; - task t([&]() -> int { - return 10; - }); + task t([&]() -> int { + return 10; + }); - t.then([&] (task ti) { - n2 = ti.get(); - }).wait(); + t.then([&] (task ti) { + n2 = ti.get(); + }).wait(); - VERIFY_IS_TRUE(n2 == 10); - } + VERIFY_IS_TRUE(n2 == 10); +} - { - int n = 0; +TEST(TestContinuationsWithTask2) +{ + int n = 0; - task tt1([](){}); - auto tt2 = tt1.then([&]()-> task { - task tt3([&](){ - n = 1; - }); - return tt3; + task tt1([](){}); + auto tt2 = tt1.then([&]()-> task { + task tt3([&](){ + n = 1; }); + return tt3; + }); - tt2.get(); - VERIFY_IS_TRUE(n == 1); + tt2.get(); + VERIFY_IS_TRUE(n == 1); - task tt4 = tt2.then([&]()-> task { - task tt5([&](){ - n = 2; - }); - return tt5; + task tt4 = tt2.then([&]()-> task { + task tt5([&](){ + n = 2; }); - tt4.get(); - VERIFY_IS_TRUE(n == 2); - } + return tt5; + }); + tt4.get(); + VERIFY_IS_TRUE(n == 2); +} - { - bool gotException = true; - int n2 = 0; - task t(ThrowFunc); - t.then([&] (task ti) { - try - { - ti.get(); - gotException = false; - } - catch (int) - { - n2 = 20; - } - }).wait(); +TEST(TestContinuationsWithTask3) +{ + bool gotException = true; + int n2 = 0; + task t(ThrowFunc); + t.then([&] (task ti) { + try + { + ti.get(); + gotException = false; + } + catch (int) + { + n2 = 20; + } + }).wait(); - VERIFY_IS_TRUE(gotException); - VERIFY_IS_TRUE(n2 == 20); - } + VERIFY_IS_TRUE(gotException); + VERIFY_IS_TRUE(n2 == 20); +} - { - int n2 = 0; +TEST(TestContinuationsWithTask4) +{ + int n2 = 0; - task t([&]() -> int { - return 10; + task t([&]() -> int { + return 10; + }); + + t.then([&] (int n) -> task { + task t2([n]() -> int { + return n+10; }); + return t2; + }).then([&](int n) { + n2 = n; + }).wait(); - t.then([&] (int n) -> task { - task t2([n]() -> int { - return n+10; - }); - return t2; - }).then([&](int n) { - n2 = n; - }).wait(); + VERIFY_IS_TRUE(n2 == 20); +} - VERIFY_IS_TRUE(n2 == 20); - } +TEST(TestContinuationsWithTask5) +{ + int n2 = 0; - { - int n2 = 0; + task t([&]() -> int { + return 10; + }); - task t([&]() -> int { - return 10; + t.then([&] (task tn) -> task { + int n = tn.get(); + task t2([n]() -> int { + return n+10; }); + return t2; + }).then([&](task n) { + n2 = n.get(); + }).wait(); - t.then([&] (task tn) -> task { - int n = tn.get(); - task t2([n]() -> int { - return n+10; - }); - return t2; - }).then([&](task n) { - n2 = n.get(); - }).wait(); - - VERIFY_IS_TRUE(n2 == 20); - } - - { - pplx::details::atomic_long hit(0); - auto * hitptr = &hit; - task t([](){ - return 10; - }); + VERIFY_IS_TRUE(n2 == 20); +} - auto ot = t.then([hitptr](int n) -> task { - auto hitptr1 = hitptr; - task it([n, hitptr1]() -> int { - os_utilities::sleep(100); - pplx::details::atomic_exchange(*hitptr1, 1L); - return n * 2; - }); +TEST(TestContinuationsWithTask6) +{ + pplx::details::atomic_long hit(0); + auto * hitptr = &hit; + task t([](){ + return 10; + }); - return it; + auto ot = t.then([hitptr](int n) -> task { + auto hitptr1 = hitptr; + task it([n, hitptr1]() -> int { + os_utilities::sleep(100); + pplx::details::atomic_exchange(*hitptr1, 1L); + return n * 2; }); - int value = ot.get(); - VERIFY_IS_TRUE(value == 20 && hit != 0); - } - - { - volatile long hit = 0; - volatile long * hitptr = &hit; + return it; + }); - task t([](){ - return 10; - }); + int value = ot.get(); + VERIFY_IS_TRUE(value == 20 && hit != 0); +} - auto ot = t.then( [hitptr](int n) -> task { - volatile long * hitptr1 = hitptr; - task it([n, hitptr1]() -> int { - os_utilities::sleep(100); - os_utilities::interlocked_exchange(hitptr1, 1); - return n * 3; - }); +TEST(TestContinuationsWithTask7) +{ + volatile long hit = 0; + volatile long * hitptr = &hit; - // This test is needed to disable an optimizer dead-code check that - // winds up generating errors in VS 2010. - if ( n == 10 ) - throw TestException1(); + task t([](){ + return 10; + }); - return it; + auto ot = t.then( [hitptr](int n) -> task { + task it([n, hitptr]() -> int { + throw TestException1(); }); - VERIFY_IS_TRUE(helpers::VerifyException(ot)); - } + return it; + }); - { - volatile long hit = 0; - volatile long * hitptr = &hit; + VERIFY_IS_TRUE(helpers::VerifyException(ot)); +} - task t([](){ - return 10; - }); +TEST(TestContinuationsWithTask8) +{ + volatile long hit = 0; + volatile long * hitptr = &hit; - auto ot = t.then( [hitptr](int n) -> task { - volatile long * hitptr1 = hitptr; - task it([n, hitptr1]() -> int { - os_utilities::sleep(100); - os_utilities::interlocked_exchange(hitptr1, 1); + task t([](){ + return 10; + }); - // This test is needed to disable an optimizer dead-code check that - // winds up generating errors in VS 2010. - if ( n == 10 ) - throw TestException2(); + auto ot = t.then( [hitptr](int n) -> task { + volatile long * hitptr1 = hitptr; + task it([n, hitptr1]() -> int { + os_utilities::sleep(100); + os_utilities::interlocked_exchange(hitptr1, 1); - return n * 3; - }); + // This test is needed to disable an optimizer dead-code check that + // winds up generating errors in VS 2010. + if ( n == 10 ) + throw TestException2(); - return it; + return n * 3; }); - VERIFY_IS_TRUE(helpers::VerifyException(ot), "(7) Inner task exception not propagated out of outer .get()"); - VERIFY_IS_TRUE(hit != 0, "(7) Expected inner task hit marker to be set!"); - } + return it; + }); - { - volatile long hit = 0; - volatile long * hitptr = &hit; - extensibility::event_t e; - task it; + VERIFY_IS_TRUE(helpers::VerifyException(ot), "(7) Inner task exception not propagated out of outer .get()"); + VERIFY_IS_TRUE(hit != 0, "(7) Expected inner task hit marker to be set!"); +} - task t([](){ - return 10; - }); +TEST(TestContinuationsWithTask9) +{ + volatile long hit = 0; + volatile long * hitptr = &hit; + extensibility::event_t e; + task it; - auto ot = t.then( [hitptr, &it, &e](int n) -> task { - volatile long * hitptr1 = hitptr; - it = task([hitptr1, n]() -> int { - os_utilities::interlocked_exchange(hitptr1, 1); - // This test is needed to disable an optimizer dead-code check that - // winds up generating errors in VS 2010. - if ( n == 10 ) - throw TestException1(); - return n * 5; - }); + task t([](){ + return 10; + }); - e.set(); - os_utilities::sleep(100); + auto ot = t.then( [hitptr, &it, &e](int n) -> task { + volatile long * hitptr1 = hitptr; + it = task([hitptr1, n]() -> int { + os_utilities::interlocked_exchange(hitptr1, 1); // This test is needed to disable an optimizer dead-code check that // winds up generating errors in VS 2010. if ( n == 10 ) - throw TestException2(); - return it; + throw TestException1(); + return n * 5; }); - e.wait(); + e.set(); + os_utilities::sleep(100); + // This test is needed to disable an optimizer dead-code check that + // winds up generating errors in VS 2010. + if ( n == 10 ) + throw TestException2(); + return it; + }); - VERIFY_IS_TRUE(helpers::VerifyException(ot), "(8) Outer task exception not propagated when inner task also throws"); - VERIFY_IS_TRUE(helpers::VerifyException(it), "(8) Inner task exception not explicitly propgated on pass out / get"); - VERIFY_IS_TRUE(hit != 0, "(8) Inner hit marker expected!"); - } + e.wait(); - { - volatile long hit = 0; + VERIFY_IS_TRUE(helpers::VerifyException(ot), "(8) Outer task exception not propagated when inner task also throws"); + VERIFY_IS_TRUE(helpers::VerifyException(it), "(8) Inner task exception not explicitly propgated on pass out / get"); + VERIFY_IS_TRUE(hit != 0, "(8) Inner hit marker expected!"); +} - task t([](){ - return 10; - }); +TEST(TestContinuationsWithTask10) +{ + volatile long hit = 0; - auto ot = t.then( [&](int n) -> task { - task it([&, n]() -> int { - os_utilities::sleep(100); - // This test is needed to disable an optimizer dead-code check that - // winds up generating errors in VS 2010. - if ( n == 10 ) - throw TestException1(); - return n * 6; - }); - return it; - }); + task t([](){ + return 10; + }); - auto otc = ot.then( [&](task itp){ - os_utilities::interlocked_exchange(&hit, 1); - VERIFY_IS_TRUE(helpers::VerifyException(itp), "(9) Outer task exception handling continuation did not get plumbed inner exception"); + auto ot = t.then( [&](int n) -> task { + task it([&, n]() -> int { + os_utilities::sleep(100); + // This test is needed to disable an optimizer dead-code check that + // winds up generating errors in VS 2010. + if ( n == 10 ) + throw TestException1(); + return n * 6; }); + return it; + }); - VERIFY_IS_TRUE(helpers::VerifyException(ot), "(9) Inner task exception not propagated correctly"); - helpers::ObserveException(otc); - VERIFY_IS_TRUE(hit != 0, "(9) Outer task exception handling continuation did not run!"); - } + auto otc = ot.then( [&](task itp){ + os_utilities::interlocked_exchange(&hit, 1); + VERIFY_IS_TRUE(helpers::VerifyException(itp), "(9) Outer task exception handling continuation did not get plumbed inner exception"); + }); + VERIFY_IS_TRUE(helpers::VerifyException(ot), "(9) Inner task exception not propagated correctly"); + helpers::ObserveException(otc); + VERIFY_IS_TRUE(hit != 0, "(9) Outer task exception handling continuation did not run!"); } TEST(TestUnwrappingCtors) @@ -1734,7 +1732,7 @@ TEST(TestUnwrappingCtors) }); }).get(); VERIFY_IS_TRUE(res==1, "unexpected value in TestUnwrappingCtors, create_task, location 1"); - + create_task([]() -> task { return create_task([]() { }); @@ -1748,10 +1746,10 @@ TEST(TestUnwrappingCtors) // Create a task that is always cancelled auto falseTask = create_task([]() {}, cts.get_token()); cancellation_token ct2 = cts2.get_token(); - create_task([falseTask]() { + create_task([falseTask]() { // Task unwrapping! // This should not crash - return falseTask; + return falseTask; }, ct2).then([this, falseTask, ct2](task t) -> task { VERIFY_IS_TRUE(t.wait() == canceled, "unexpected value in TestUnwrappingCtors, cancellation token, location 1"); VERIFY_IS_TRUE(!ct2.is_canceled(), "unexpected value in TestUnwrappingCtors, cancellation token, location 2"); @@ -1776,7 +1774,7 @@ TEST(TestNestedTasks) return task([=]() -> int { return val1 + 22; }); - }); + }); }); int n = resultTask.get().get(); @@ -1789,17 +1787,17 @@ TEST(TestNestedTasks) int * flagptr = &flag; task rootTask([&]() { flag++; }); - task> resultTask = rootTask.then([flagptr]() -> task> + task> resultTask = rootTask.then([flagptr]() -> task> { auto flag1 = flagptr; - return task>([flag1]() -> task + return task>([flag1]() -> task { auto flag2 = flag1; - return task([flag2]() + return task([flag2]() { ++(flag2[0]); }); - }); + }); }); resultTask.get().wait(); @@ -1809,7 +1807,7 @@ TEST(TestNestedTasks) { task rootTask([]() -> int { return 234; }); - task>> + task>> resultTask = rootTask.then([](int value) -> task>> { return task>>([=]() -> task> { auto v1 = value; @@ -1818,7 +1816,7 @@ TEST(TestNestedTasks) return task([=]() -> int { return v2 + 22; }); - }); + }); }); }); From 8179d49a6c0a20f5075100538aa9c8104e9fed4b Mon Sep 17 00:00:00 2001 From: Billy Robert O'Neal III Date: Wed, 17 Oct 2018 18:52:09 -0700 Subject: [PATCH 2/5] Fix several errors found by thread sanitizer. --- Release/include/pplx/pplxlinux.h | 13 ++- Release/include/pplx/pplxtasks.h | 105 +++++++++--------- .../common/UnitTestpp/src/CurrentTest.cpp | 46 +++++--- .../tests/common/UnitTestpp/src/CurrentTest.h | 32 +++--- .../tests/common/UnitTestpp/src/ExecuteTest.h | 28 ++--- .../common/UnitTestpp/src/TestRunner.cpp | 44 ++++---- 6 files changed, 141 insertions(+), 127 deletions(-) diff --git a/Release/include/pplx/pplxlinux.h b/Release/include/pplx/pplxlinux.h index 78b6e1801a..0365f83a30 100644 --- a/Release/include/pplx/pplxlinux.h +++ b/Release/include/pplx/pplxlinux.h @@ -29,7 +29,8 @@ #include #include #else -#include +#include +#include #include #endif @@ -80,7 +81,7 @@ namespace platform static const unsigned int timeout_infinite = 0xFFFFFFFF; event_impl() - : _signaled(false) + : _signaled(false) { } @@ -209,7 +210,7 @@ namespace platform _M_cs.lock(); _M_owner = id; _M_recursionCount = 1; - } + } } void unlock() @@ -223,12 +224,12 @@ namespace platform { _M_owner = -1; _M_cs.unlock(); - } + } } private: cpprest_synchronization::mutex _M_cs; - volatile long _M_owner; + std::atomic _M_owner; long _M_recursionCount; }; @@ -298,7 +299,7 @@ namespace extensibility #else typedef details::linux_scheduler default_scheduler_t; #endif - + namespace details { /// diff --git a/Release/include/pplx/pplxtasks.h b/Release/include/pplx/pplxtasks.h index e784306d40..0c337d5a5e 100644 --- a/Release/include/pplx/pplxtasks.h +++ b/Release/include/pplx/pplxtasks.h @@ -58,6 +58,7 @@ void cpprest_init(JavaVM*); #include #include #include +#include #if defined(_MSC_VER) #include @@ -175,7 +176,7 @@ template <> class task; #endif /// -/// Helper macro to determine how many stack frames need to be saved. When any number less or equal to 1 is specified, +/// Helper macro to determine how many stack frames need to be saved. When any number less or equal to 1 is specified, /// only one frame is captured and no stackwalk will be involved. Otherwise, the number of callstack frames will be captured. /// /// @@ -270,7 +271,7 @@ namespace details { _TaskCreationCallstack _csc; _csc._M_frames.resize(_CaptureFrames); - // skip 2 frames to make sure callstack starts from user code + // skip 2 frames to make sure callstack starts from user code _csc._M_frames.resize(::pplx::details::platform::CaptureCallstack(&_csc._M_frames[0], 2, _CaptureFrames)); return _csc; } @@ -439,10 +440,10 @@ namespace details template task<_Type> _To_task(_Type t); - + template task _To_task_void(_Func f); - + struct _BadContinuationParamType{}; template auto _ReturnTypeHelper(_Type t, _Function _Func, int, int) -> decltype(_Func(_To_task(t))); @@ -1322,7 +1323,7 @@ class task_options _M_CancellationToken(_TaskOptions.get_cancellation_token()), _M_ContinuationContext(_TaskOptions.get_continuation_context()), _M_HasCancellationToken(_TaskOptions.has_cancellation_token()), - _M_HasScheduler(_TaskOptions.has_scheduler()) + _M_HasScheduler(_TaskOptions.has_scheduler()) { } @@ -1430,10 +1431,10 @@ namespace details // This field gives inlining scheduling policy for current chore. _TaskInliningMode_t _M_inliningMode; - + virtual _Task_ptr_base _GetTaskImplBase() const = 0; - _ContinuationTaskHandleBase() : + _ContinuationTaskHandleBase() : _M_next(nullptr), _M_continuationContext(task_continuation_context::use_default()), _M_isTaskBasedContinuation(false), _M_inliningMode(details::_NoInline) { } @@ -1464,7 +1465,7 @@ namespace details VER_SET_CONDITION( _conditionMask, VER_MAJORVERSION, VER_GREATER_EQUAL ); VER_SET_CONDITION( _conditionMask, VER_MINORVERSION, VER_GREATER_EQUAL ); - if ( ::VerifyVersionInfo(&_osvi, VER_MAJORVERSION | VER_MINORVERSION, _conditionMask)) + if ( ::VerifyVersionInfo(&_osvi, VER_MAJORVERSION | VER_MINORVERSION, _conditionMask)) { _causality = 2; } @@ -1479,7 +1480,7 @@ namespace details #endif } - // Stateful logger rests inside task_impl_base. + // Stateful logger rests inside task_impl_base. struct _TaskEventLogger { _Task_impl_base *_M_task; @@ -1491,11 +1492,11 @@ namespace details { if (details::_IsCausalitySupported()) { - ::Windows::Foundation::Diagnostics::AsyncCausalityTracer::TraceOperationCreation(::Windows::Foundation::Diagnostics::CausalityTraceLevel::Required, ::Windows::Foundation::Diagnostics::CausalitySource::Library, - _PPLTaskCausalityPlatformID, reinterpret_cast(_M_task), + ::Windows::Foundation::Diagnostics::AsyncCausalityTracer::TraceOperationCreation(::Windows::Foundation::Diagnostics::CausalityTraceLevel::Required, ::Windows::Foundation::Diagnostics::CausalitySource::Library, + _PPLTaskCausalityPlatformID, reinterpret_cast(_M_task), _isContinuation ? "pplx::PPLTask::ScheduleContinuationTask" : "pplx::PPLTask::ScheduleTask", 0); _M_scheduled = true; - } + } } // It will log the cancel event but not canceled state. _LogTaskCompleted will log the terminal state, which includes cancel state. @@ -1506,7 +1507,7 @@ namespace details ::Windows::Foundation::Diagnostics::AsyncCausalityTracer::TraceOperationRelation(::Windows::Foundation::Diagnostics::CausalityTraceLevel::Important, ::Windows::Foundation::Diagnostics::CausalitySource::Library, _PPLTaskCausalityPlatformID, reinterpret_cast(_M_task), ::Windows::Foundation::Diagnostics::CausalityRelation::Cancel); - } + } } // Log when task reaches terminal state. Note: the task can reach a terminal state (by cancellation or exception) without having run @@ -1548,7 +1549,7 @@ namespace details _M_taskPostEventStarted = true; } } - + _TaskEventLogger(_Task_impl_base *_task): _M_task(_task) { _M_scheduled = false; @@ -1613,7 +1614,7 @@ namespace details { } - virtual ~_PPLTaskHandle() + virtual ~_PPLTaskHandle() { // Here is the sink of all task completion code paths _M_pTask->_M_taskEventLogger._LogTaskCompleted(); @@ -1690,11 +1691,11 @@ namespace details #pragma warning(push) #pragma warning(disable: 4355) #endif - _Task_impl_base(_CancellationTokenState * _PTokenState, scheduler_ptr _Scheduler_arg) + _Task_impl_base(_CancellationTokenState * _PTokenState, scheduler_ptr _Scheduler_arg) : _M_TaskState(_Created), _M_fFromAsync(false), _M_fUnwrappedTask(false), _M_pRegistration(nullptr), _M_Continuations(nullptr), _M_TaskCollection(_Scheduler_arg), - _M_taskEventLogger(this) + _M_taskEventLogger(this) { // Set cancelation token _M_pTokenState = _PTokenState; @@ -2041,7 +2042,7 @@ namespace details // Schedule a continuation to run void _ScheduleContinuationTask(_ContinuationTaskHandleBase * _PTaskHandle) { - + _M_taskEventLogger._LogScheduleTask(true); // Ensure that the continuation runs in proper context (this might be on a Concurrency Runtime thread or in a different Windows Runtime apartment) if (_PTaskHandle->_M_continuationContext._HasCapturedContext()) @@ -2286,7 +2287,7 @@ namespace details static void _AsyncInit(const typename _Task_ptr<_ReturnType>::_Type& _OuterTask, const task<_InternalReturnType> & _UnwrappedTask) { _ASSERTE(_OuterTask->_M_fUnwrappedTask && !_OuterTask->_IsCanceled()); - + // // We must ensure that continuations off _OuterTask (especially exception handling ones) continue to function in the // presence of an exception flowing out of the inner task _UnwrappedTask. This requires an exception handling continuation @@ -2324,7 +2325,7 @@ namespace details } // Tracks the internal state of the task - volatile _TaskInternalState _M_TaskState; + std::atomic<_TaskInternalState> _M_TaskState; // Set to true either if the ancestor task had the flag set to true, or if the lambda that does the work of this task returns an // async operation or async action that is unwrapped by the runtime. bool _M_fFromAsync; @@ -2381,7 +2382,7 @@ namespace details } } #endif - + /// /// The implementation of a first-class task. This structure contains the task group used to execute /// the task function and handles the scheduling. The _Task_impl is created as a shared_ptr @@ -2455,7 +2456,7 @@ namespace details if (_SynchronousCancel) { // Be aware that this set must be done BEFORE _M_Scheduled being set, or race will happen between this and wait() - _M_TaskState = _Canceled; + _M_TaskState = _Canceled; // Cancellation completes the task, so all dependent tasks must be run to cancel them // They are canceled when they begin running (see _RunContinuation) and see that their // ancestor has been canceled. @@ -2485,7 +2486,7 @@ namespace details _M_taskEventLogger._LogCancelTask(); } - + } // Only execute continuations and mark the task as completed if we were able to move the task to the _Canceled state. @@ -2563,7 +2564,7 @@ namespace details // Return true if the task has reached a terminal state bool _IsDone() - { + { return _IsCompleted() || _IsCanceled(); } @@ -2613,9 +2614,9 @@ namespace details _TaskList _M_tasks; ::pplx::extensibility::critical_section_t _M_taskListCritSec; _ResultHolder<_ResultType> _M_value; - std::shared_ptr<_ExceptionHolder> _M_exceptionHolder; - bool _M_fHasValue; - bool _M_fIsCanceled; + std::shared_ptr<_ExceptionHolder> _M_exceptionHolder; + std::atomic _M_fHasValue; + std::atomic _M_fIsCanceled; }; // Utility method for dealing with void functions @@ -2667,8 +2668,8 @@ class task_completion_event /// Constructs a task_completion_event object. /// /**/ - task_completion_event() - : _M_Impl(std::make_shared>()) + task_completion_event() + : _M_Impl(std::make_shared>()) { } @@ -3334,7 +3335,7 @@ class task _CreateImpl(_TaskOptions.get_cancellation_token()._GetImplValue(), _TaskOptions.get_scheduler()); // Do not move the next line out of this function. It is important that _CAPTURE_CALLSTACK() evaluate to the the call site of the task constructor. _SetTaskCreationCallstack(details::_get_internal_task_options(_TaskOptions)._M_hasPresetCreationCallstack ? details::_get_internal_task_options(_TaskOptions)._M_presetCreationCallstack : _CAPTURE_CALLSTACK()); - + _TaskInitMaybeFunctor(_Param, details::_IsCallable(_Param,0)); } @@ -3570,7 +3571,7 @@ class task if (_M_Impl->_Wait() == canceled) { - throw task_canceled(); + throw task_canceled(); } return _M_Impl->_GetResult(); @@ -3712,7 +3713,7 @@ class task /// This function is Used for runtime internal continuations only. /// template - auto _Then(_Function&& _Func, details::_CancellationTokenState *_PTokenState, + auto _Then(_Function&& _Func, details::_CancellationTokenState *_PTokenState, details::_TaskInliningMode_t _InliningMode = details::_ForceInline) const -> typename details::_ContinuationTypeTraits<_Function, _ReturnType>::_TaskOfType { // inherit from antecedent @@ -3724,10 +3725,10 @@ class task private: template friend class task; - + // The task handle type used to construct an 'initial task' - a task with no dependents. template - struct _InitialTaskHandle : + struct _InitialTaskHandle : details::_PPLTaskHandle<_ReturnType, _InitialTaskHandle<_InternalReturnType, _Function, _TypeSelection>, details::_UnrealizedChore_t> { _Function _M_function; @@ -3813,7 +3814,7 @@ class task { typedef details::_GetProgressType::_Value _ProgressType; - details::_Task_impl_base::_AsyncInit<_ReturnType, _InternalReturnType>(this->_M_pTask, + details::_Task_impl_base::_AsyncInit<_ReturnType, _InternalReturnType>(this->_M_pTask, ref new details::_IAsyncActionWithProgressToAsyncOperationConverter<_ProgressType>(_LogWorkItemAndInvokeUserLambda(_M_function))); } #endif /* defined (__cplusplus_winrt) */ @@ -3904,7 +3905,7 @@ class task typedef typename details::_FunctionTypeTraits<_Function, _InternalReturnType>::_FuncRetType _FuncOutputType; details::_Task_impl_base::_AsyncInit<_NormalizedContinuationReturnType, _ContinuationReturnType>( - this->_M_pTask, + this->_M_pTask, _LogWorkItemAndInvokeUserLambda(_Continuation_func_transformer<_InternalReturnType, _FuncOutputType>::_Perform(_M_function), _M_ancestorTaskImpl->_GetResult()) ); } @@ -3989,7 +3990,7 @@ class task // The continuation takes a parameter of type task<_Input>, which is the same as the ancestor task. task<_InternalReturnType> _ResultTask; _ResultTask._SetImpl(std::move(_M_ancestorTaskImpl)); - details::_Task_impl_base::_AsyncInit<_NormalizedContinuationReturnType, _ContinuationReturnType>(this->_M_pTask, + details::_Task_impl_base::_AsyncInit<_NormalizedContinuationReturnType, _ContinuationReturnType>(this->_M_pTask, _LogWorkItemAndInvokeUserLambda(_M_function, std::move(_ResultTask))); } @@ -4586,7 +4587,7 @@ class task /// An internal version of then that takes additional flags and executes the continuation inline. Used for runtime internal continuations only. /// template - auto _Then(_Function&& _Func, details::_CancellationTokenState *_PTokenState, + auto _Then(_Function&& _Func, details::_CancellationTokenState *_PTokenState, details::_TaskInliningMode_t _InliningMode = details::_ForceInline) const -> typename details::_ContinuationTypeTraits<_Function, void>::_TaskOfType { // inherit from antecedent @@ -5651,8 +5652,8 @@ namespace details { internal: - _AsyncInfoBase() : - _M_currentStatus(_AsyncStatusInternal::_AsyncCreated), + _AsyncInfoBase() : + _M_currentStatus(_AsyncStatusInternal::_AsyncCreated), _M_errorCode(S_OK), _M_completeDelegate(nullptr), _M_CompleteDelegateAssigned(0), @@ -6296,7 +6297,7 @@ namespace details void _Resize(size_t _Len, bool _SkipVector = false) { _M_numTasks = _Len; - + if (!_SkipVector) { _M_vector.resize(_Len); @@ -6433,7 +6434,7 @@ namespace details }; _WhenAllContinuationWrapper(_PParam, _Func, _ResultTask); - }, _CancellationTokenState::_None()); + }, _CancellationTokenState::_None()); _Index++; } @@ -6598,7 +6599,7 @@ namespace details auto _mergeVal = _PParam->_M_mergeVal.Get(); if (_OutputVectorFirst == true) - { + { _Result.push_back(_mergeVal); } else @@ -6666,7 +6667,7 @@ namespace details /// /**/ template -auto when_all(_Iterator _Begin, _Iterator _End, const task_options& _TaskOptions = task_options()) +auto when_all(_Iterator _Begin, _Iterator _End, const task_options& _TaskOptions = task_options()) -> decltype (details::_WhenAllImpl::value_type::result_type, _Iterator>::_Perform(_TaskOptions, _Begin, _End)) { typedef typename std::iterator_traits<_Iterator>::value_type::result_type _ElementType; @@ -6850,7 +6851,7 @@ namespace details } } } - + if (atomic_increment(_PParam->_M_completeCount) == _PParam->_M_numTasks) { // If no one has be completed so far, we need to make some final cancellation decision. @@ -6889,16 +6890,16 @@ namespace details } _CancellationTokenState *_PTokenState = _TaskOptions.has_cancellation_token() ? _TaskOptions.get_cancellation_token()._GetImplValue() : nullptr; auto _PParam = new _RunAnyParam, _CancellationTokenState *>>(); - + if (_PTokenState) { _JoinAllTokens_Add(_PParam->_M_cancellationSource, _PTokenState); _PParam->_M_fHasExplicitToken = true; } - + task_options _Options(_TaskOptions); _Options.set_cancellation_token(_PParam->_M_cancellationSource.get_token()); - task, _CancellationTokenState *>> _Any_tasks_completed(_PParam->_M_Completed, _Options); + task, _CancellationTokenState *>> _Any_tasks_completed(_PParam->_M_Completed, _Options); // Keep a copy ref to the token source auto _CancellationSource = _PParam->_M_cancellationSource; @@ -6949,7 +6950,7 @@ namespace details _CancellationTokenState *_PTokenState = _TaskOptions.has_cancellation_token() ? _TaskOptions.get_cancellation_token()._GetImplValue() : nullptr; auto _PParam = new _RunAnyParam>(); - + if (_PTokenState) { _JoinAllTokens_Add(_PParam->_M_cancellationSource, _PTokenState); @@ -7175,8 +7176,8 @@ task> operator||(const task> & _WhenAnyContinuationWrapper(_PParam, _Func, _ResultTask); }, details::_CancellationTokenState::_None()); - - _Rhs._Then([_PParam](task<_ReturnType> _ResultTask) + + _Rhs._Then([_PParam](task<_ReturnType> _ResultTask) { auto _PParamCopy = _PParam; auto _Func = [&_ResultTask, _PParamCopy]() { @@ -7332,5 +7333,3 @@ namespace concurrency = Concurrency; #endif #endif // _PPLXTASKS_H - - diff --git a/Release/tests/common/UnitTestpp/src/CurrentTest.cpp b/Release/tests/common/UnitTestpp/src/CurrentTest.cpp index 08916e9d2f..dae85da8d4 100644 --- a/Release/tests/common/UnitTestpp/src/CurrentTest.cpp +++ b/Release/tests/common/UnitTestpp/src/CurrentTest.cpp @@ -1,10 +1,10 @@ /*** * This file is based on or incorporates material from the UnitTest++ r30 open source project. -* Microsoft is not the original author of this code but has modified it and is licensing the code under -* the MIT License. Microsoft reserves all other rights not expressly granted under the MIT License, -* whether by implication, estoppel or otherwise. +* Microsoft is not the original author of this code but has modified it and is licensing the code under +* the MIT License. Microsoft reserves all other rights not expressly granted under the MIT License, +* whether by implication, estoppel or otherwise. * -* UnitTest++ r30 +* UnitTest++ r30 * * Copyright (c) 2006 Noel Llopis and Charles Nicholson * Portions Copyright (c) Microsoft Corporation @@ -13,36 +13,48 @@ * * MIT License * -* Permission is hereby granted, free of charge, to any person obtaining a copy of this software -* and associated documentation files (the "Software"), to deal in the Software without restriction, -* including without limitation the rights to use, copy, modify, merge, publish, distribute, sublicense, -* and/or sell copies of the Software, and to permit persons to whom the Software is furnished to do so, +* Permission is hereby granted, free of charge, to any person obtaining a copy of this software +* and associated documentation files (the "Software"), to deal in the Software without restriction, +* including without limitation the rights to use, copy, modify, merge, publish, distribute, sublicense, +* and/or sell copies of the Software, and to permit persons to whom the Software is furnished to do so, * subject to the following conditions: * -* The above copyright notice and this permission notice shall be included in all copies or +* The above copyright notice and this permission notice shall be included in all copies or * substantial portions of the Software. * -* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLIED, -* INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE -* AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, -* DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLIED, +* INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE +* AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, +* DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. ***/ #include "stdafx.h" +#include + +namespace { + std::atomic testResults; + std::atomic testDetails; +} namespace UnitTest { -UNITTEST_LINKAGE TestResults*& CurrentTest::Results() +UNITTEST_LINKAGE TestResults* CurrentTest::Results() { - static TestResults* testResults = NULL; return testResults; } -UNITTEST_LINKAGE const TestDetails*& CurrentTest::Details() +UNITTEST_LINKAGE void CurrentTest::SetResults(TestResults * r) { + testResults.store(r); +} + +UNITTEST_LINKAGE const TestDetails* CurrentTest::Details() { - static const TestDetails* testDetails = NULL; return testDetails; } +UNITTEST_LINKAGE void CurrentTest::SetDetails(const UnitTest::TestDetails * d) { + testDetails.store(d); +} + } diff --git a/Release/tests/common/UnitTestpp/src/CurrentTest.h b/Release/tests/common/UnitTestpp/src/CurrentTest.h index 0b64ade5e7..4dbe6eff94 100644 --- a/Release/tests/common/UnitTestpp/src/CurrentTest.h +++ b/Release/tests/common/UnitTestpp/src/CurrentTest.h @@ -1,10 +1,10 @@ /*** * This file is based on or incorporates material from the UnitTest++ r30 open source project. -* Microsoft is not the original author of this code but has modified it and is licensing the code under -* the MIT License. Microsoft reserves all other rights not expressly granted under the MIT License, -* whether by implication, estoppel or otherwise. +* Microsoft is not the original author of this code but has modified it and is licensing the code under +* the MIT License. Microsoft reserves all other rights not expressly granted under the MIT License, +* whether by implication, estoppel or otherwise. * -* UnitTest++ r30 +* UnitTest++ r30 * * Copyright (c) 2006 Noel Llopis and Charles Nicholson * Portions Copyright (c) Microsoft Corporation @@ -13,19 +13,19 @@ * * MIT License * -* Permission is hereby granted, free of charge, to any person obtaining a copy of this software -* and associated documentation files (the "Software"), to deal in the Software without restriction, -* including without limitation the rights to use, copy, modify, merge, publish, distribute, sublicense, -* and/or sell copies of the Software, and to permit persons to whom the Software is furnished to do so, +* Permission is hereby granted, free of charge, to any person obtaining a copy of this software +* and associated documentation files (the "Software"), to deal in the Software without restriction, +* including without limitation the rights to use, copy, modify, merge, publish, distribute, sublicense, +* and/or sell copies of the Software, and to permit persons to whom the Software is furnished to do so, * subject to the following conditions: * -* The above copyright notice and this permission notice shall be included in all copies or +* The above copyright notice and this permission notice shall be included in all copies or * substantial portions of the Software. * -* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLIED, -* INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE -* AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, -* DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLIED, +* INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE +* AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, +* DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. ***/ @@ -41,8 +41,10 @@ class TestDetails; namespace CurrentTest { - UNITTEST_LINKAGE TestResults*& __cdecl Results(); - UNITTEST_LINKAGE const TestDetails*& __cdecl Details(); + UNITTEST_LINKAGE TestResults* __cdecl Results(); + UNITTEST_LINKAGE void __cdecl SetResults(TestResults*); + UNITTEST_LINKAGE const TestDetails* __cdecl Details(); + UNITTEST_LINKAGE void __cdecl SetDetails(const TestDetails *); } } diff --git a/Release/tests/common/UnitTestpp/src/ExecuteTest.h b/Release/tests/common/UnitTestpp/src/ExecuteTest.h index 8cd5ed74b3..ef87c48dc3 100644 --- a/Release/tests/common/UnitTestpp/src/ExecuteTest.h +++ b/Release/tests/common/UnitTestpp/src/ExecuteTest.h @@ -1,10 +1,10 @@ /*** * This file is based on or incorporates material from the UnitTest++ r30 open source project. -* Microsoft is not the original author of this code but has modified it and is licensing the code under -* the MIT License. Microsoft reserves all other rights not expressly granted under the MIT License, -* whether by implication, estoppel or otherwise. +* Microsoft is not the original author of this code but has modified it and is licensing the code under +* the MIT License. Microsoft reserves all other rights not expressly granted under the MIT License, +* whether by implication, estoppel or otherwise. * -* UnitTest++ r30 +* UnitTest++ r30 * * Copyright (c) 2006 Noel Llopis and Charles Nicholson * Portions Copyright (c) Microsoft Corporation @@ -13,19 +13,19 @@ * * MIT License * -* Permission is hereby granted, free of charge, to any person obtaining a copy of this software -* and associated documentation files (the "Software"), to deal in the Software without restriction, -* including without limitation the rights to use, copy, modify, merge, publish, distribute, sublicense, -* and/or sell copies of the Software, and to permit persons to whom the Software is furnished to do so, +* Permission is hereby granted, free of charge, to any person obtaining a copy of this software +* and associated documentation files (the "Software"), to deal in the Software without restriction, +* including without limitation the rights to use, copy, modify, merge, publish, distribute, sublicense, +* and/or sell copies of the Software, and to permit persons to whom the Software is furnished to do so, * subject to the following conditions: * -* The above copyright notice and this permission notice shall be included in all copies or +* The above copyright notice and this permission notice shall be included in all copies or * substantial portions of the Software. * -* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLIED, -* INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE -* AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, -* DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLIED, +* INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE +* AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, +* DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. ***/ @@ -57,7 +57,7 @@ void ExecuteTest(T& testObject, TestDetails const& details, bool isMockTest) { if (isMockTest == false) { - CurrentTest::Details() = &details; + CurrentTest::SetDetails(&details); } #ifdef UNITTEST_NO_EXCEPTIONS diff --git a/Release/tests/common/UnitTestpp/src/TestRunner.cpp b/Release/tests/common/UnitTestpp/src/TestRunner.cpp index 6b249404b7..3f523cf9a8 100644 --- a/Release/tests/common/UnitTestpp/src/TestRunner.cpp +++ b/Release/tests/common/UnitTestpp/src/TestRunner.cpp @@ -1,10 +1,10 @@ /*** * This file is based on or incorporates material from the UnitTest++ r30 open source project. -* Microsoft is not the original author of this code but has modified it and is licensing the code under -* the MIT License. Microsoft reserves all other rights not expressly granted under the MIT License, -* whether by implication, estoppel or otherwise. +* Microsoft is not the original author of this code but has modified it and is licensing the code under +* the MIT License. Microsoft reserves all other rights not expressly granted under the MIT License, +* whether by implication, estoppel or otherwise. * -* UnitTest++ r30 +* UnitTest++ r30 * * Copyright (c) 2006 Noel Llopis and Charles Nicholson * Portions Copyright (c) Microsoft Corporation @@ -13,19 +13,19 @@ * * MIT License * -* Permission is hereby granted, free of charge, to any person obtaining a copy of this software -* and associated documentation files (the "Software"), to deal in the Software without restriction, -* including without limitation the rights to use, copy, modify, merge, publish, distribute, sublicense, -* and/or sell copies of the Software, and to permit persons to whom the Software is furnished to do so, +* Permission is hereby granted, free of charge, to any person obtaining a copy of this software +* and associated documentation files (the "Software"), to deal in the Software without restriction, +* including without limitation the rights to use, copy, modify, merge, publish, distribute, sublicense, +* and/or sell copies of the Software, and to permit persons to whom the Software is furnished to do so, * subject to the following conditions: * -* The above copyright notice and this permission notice shall be included in all copies or +* The above copyright notice and this permission notice shall be included in all copies or * substantial portions of the Software. * -* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLIED, -* INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE -* AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, -* DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLIED, +* INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE +* AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, +* DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. ***/ @@ -78,11 +78,11 @@ TestResults* TestRunner::GetTestResults() int TestRunner::Finish() const { float const secondsElapsed = static_cast(m_timer->GetTimeInMs() / 1000.0); - m_reporter->ReportSummary(m_result->GetTotalTestCount(), - m_result->GetFailedTestCount(), - m_result->GetFailureCount(), + m_reporter->ReportSummary(m_result->GetTotalTestCount(), + m_result->GetFailedTestCount(), + m_result->GetFailureCount(), secondsElapsed); - + return m_result->GetFailureCount(); } @@ -97,8 +97,8 @@ bool TestRunner::IsTestInSuite(const Test* const curTest, char const* suiteName) // to use agent. class TestRunnerAgent : public Concurrency::agent { -public: - TestRunnerAgent(std::tr1::function func) : m_func(func) {} +public: + TestRunnerAgent(std::tr1::function func) : m_func(func) {} protected: void run() { @@ -137,7 +137,7 @@ int TestRunner::GetTestTimeout(Test* const curTest, int const defaultTestTimeInM void TestRunner::RunTest(TestResults* const result, Test* const curTest, int const defaultTestTimeInMs) const { if (curTest->m_isMockTest == false) - CurrentTest::Results() = result; + CurrentTest::SetResults(result); int maxTestTimeInMs = GetTestTimeout(curTest, defaultTestTimeInMs); @@ -158,7 +158,7 @@ void TestRunner::RunTest(TestResults* const result, Test* const curTest, int con try { Concurrency::agent::wait(&testRunnerAgent, maxTestTimeInMs); - } + } catch(const Concurrency::operation_timed_out &) { timedOut = true; @@ -196,7 +196,7 @@ void TestRunner::RunTest(TestResults* const result, Test* const curTest, int con abort(); } - } + } else { curTest->Run(); From c1676ce5b15a2915082a8266dd7f4e147ab19118 Mon Sep 17 00:00:00 2001 From: Billy Robert O'Neal III Date: Wed, 17 Oct 2018 19:52:07 -0700 Subject: [PATCH 3/5] Fix more data races :/ --- Release/include/cpprest/astreambuf.h | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/Release/include/cpprest/astreambuf.h b/Release/include/cpprest/astreambuf.h index baff9a934d..872055b463 100644 --- a/Release/include/cpprest/astreambuf.h +++ b/Release/include/cpprest/astreambuf.h @@ -13,10 +13,11 @@ ****/ #pragma once +#include +#include +#include #include #include -#include -#include #include "pplx/pplxtasks.h" #include "cpprest/details/basic_types.h" @@ -745,7 +746,10 @@ namespace streams std::exception_ptr m_currentException; // The in/out mode for the buffer - bool m_stream_can_read, m_stream_can_write, m_stream_read_eof, m_alloced; + std::atomic m_stream_can_read; + std::atomic m_stream_can_write; + std::atomic m_stream_read_eof; + std::atomic m_alloced; private: @@ -1208,5 +1212,3 @@ namespace streams }; }} - - From c5466cccd920d5b04d67d5fd05c6b93839bdb6fd Mon Sep 17 00:00:00 2001 From: Billy Robert O'Neal III Date: Wed, 17 Oct 2018 20:28:42 -0700 Subject: [PATCH 4/5] Fix data races in the asio server found by tsan. --- .../src/http/listener/http_server_asio.cpp | 97 +++++++++++-------- 1 file changed, 57 insertions(+), 40 deletions(-) diff --git a/Release/src/http/listener/http_server_asio.cpp b/Release/src/http/listener/http_server_asio.cpp index 591e623d82..02aa616177 100644 --- a/Release/src/http/listener/http_server_asio.cpp +++ b/Release/src/http/listener/http_server_asio.cpp @@ -320,7 +320,19 @@ class asio_server_connection boost::asio::streambuf m_response_buf; http_linux_server* m_p_server; hostport_listener* m_p_parent; - http_request m_request; + mutable std::mutex m_request_mtx; + http_request m_request_tmp; + + void set_request(http_request req) { + std::lock_guard lck(m_request_mtx); + m_request_tmp = std::move(req); + } + + http_request get_request() const { + std::lock_guard lck(m_request_mtx); + return m_request_tmp; + } + size_t m_read, m_write; size_t m_read_size, m_write_size; bool m_close; @@ -409,7 +421,7 @@ class asio_server_connection will_erase_from_parent_t do_response() { auto unique_reference = this->get_reference(); - m_request.get_response().then([=](pplx::task r_task) + get_request().get_response().then([=](pplx::task r_task) { http_response response; try @@ -424,7 +436,7 @@ class asio_server_connection serialize_headers(response); // before sending response, the full incoming message need to be processed. - return m_request.content_ready().then([=](pplx::task) + return get_request().content_ready().then([=](pplx::task) { (will_deref_and_erase_t)this->async_write(&asio_server_connection::handle_headers_written, response); }); @@ -435,7 +447,7 @@ class asio_server_connection will_erase_from_parent_t do_bad_response() { auto unique_reference = this->get_reference(); - m_request.get_response().then([=](pplx::task r_task) + get_request().get_response().then([=](pplx::task r_task) { http_response response; try @@ -540,7 +552,7 @@ void asio_server_connection::close() sock->shutdown(tcp::socket::shutdown_both, ec); sock->close(ec); } - m_request._reply_if_not_already(status_codes::InternalError); + get_request()._reply_if_not_already(status_codes::InternalError); } will_deref_and_erase_t asio_server_connection::start_request_response() @@ -617,7 +629,8 @@ void hostport_listener::on_accept(std::unique_ptr socket, const will_deref_and_erase_t asio_server_connection::handle_http_line(const boost::system::error_code& ec) { - m_request = http_request::_create_request(make_unique()); + auto thisRequest = http_request::_create_request(make_unique()); + set_request(thisRequest); if (ec) { // client closed connection @@ -632,7 +645,7 @@ will_deref_and_erase_t asio_server_connection::handle_http_line(const boost::sys } else { - m_request._reply_if_not_already(status_codes::BadRequest); + thisRequest._reply_if_not_already(status_codes::BadRequest); m_close = true; (will_erase_from_parent_t)do_bad_response(); (will_deref_t)deref(); @@ -669,14 +682,14 @@ will_deref_and_erase_t asio_server_connection::handle_http_line(const boost::sys // Check to see if there is not allowed character on the input if (!web::http::details::validate_method(http_verb)) { - m_request.reply(status_codes::BadRequest); + thisRequest.reply(status_codes::BadRequest); m_close = true; (will_erase_from_parent_t)do_bad_response(); (will_deref_t)deref(); return will_deref_and_erase_t{}; } - m_request.set_method(http_verb); + thisRequest.set_method(http_verb); std::string http_path_and_version; std::getline(request_stream, http_path_and_version); @@ -685,7 +698,7 @@ will_deref_and_erase_t asio_server_connection::handle_http_line(const boost::sys // Make sure path and version is long enough to contain the HTTP version if(http_path_and_version.size() < VersionPortionSize + 2) { - m_request.reply(status_codes::BadRequest); + thisRequest.reply(status_codes::BadRequest); m_close = true; (will_erase_from_parent_t)do_bad_response(); (will_deref_t)deref(); @@ -695,12 +708,12 @@ will_deref_and_erase_t asio_server_connection::handle_http_line(const boost::sys // Get the path - remove the version portion and prefix space try { - m_request.set_request_uri(utility::conversions::to_string_t( + thisRequest.set_request_uri(utility::conversions::to_string_t( http_path_and_version.substr(1, http_path_and_version.size() - VersionPortionSize - 1))); } catch (const std::exception& e) // may be std::range_error indicating invalid Unicode, or web::uri_exception { - m_request.reply(status_codes::BadRequest, e.what()); + thisRequest.reply(status_codes::BadRequest, e.what()); m_close = true; (will_erase_from_parent_t)do_bad_response(); (will_deref_t)deref(); @@ -710,9 +723,9 @@ will_deref_and_erase_t asio_server_connection::handle_http_line(const boost::sys // Get the version std::string http_version = http_path_and_version.substr(http_path_and_version.size() - VersionPortionSize + 1, VersionPortionSize - 2); - auto m_request_impl = m_request._get_impl().get(); + auto requestImpl = thisRequest._get_impl().get(); web::http::http_version parsed_version = web::http::http_version::from_string(http_version); - m_request_impl->_set_http_version(parsed_version); + requestImpl->_set_http_version(parsed_version); // if HTTP version is 1.0 then disable pipelining if (parsed_version == web::http::http_versions::HTTP_1_0) @@ -725,7 +738,7 @@ will_deref_and_erase_t asio_server_connection::handle_http_line(const boost::sys auto endpoint = m_socket->remote_endpoint(socket_ec); if (!socket_ec) { - m_request._get_impl()->_set_remote_address(utility::conversions::to_string_t( + requestImpl->_set_remote_address(utility::conversions::to_string_t( endpoint.address().to_string())); } @@ -739,7 +752,8 @@ will_deref_and_erase_t asio_server_connection::handle_headers() request_stream.imbue(std::locale::classic()); std::string header; - auto& headers = m_request.headers(); + auto currentRequest = get_request(); + auto& headers = currentRequest.headers(); while (std::getline(request_stream, header) && header != "\r") { @@ -763,7 +777,7 @@ will_deref_and_erase_t asio_server_connection::handle_headers() } else { - m_request.reply(status_codes::BadRequest); + currentRequest.reply(status_codes::BadRequest); m_close = true; (will_erase_from_parent_t)do_bad_response(); (will_deref_t)deref(); @@ -774,17 +788,17 @@ will_deref_and_erase_t asio_server_connection::handle_headers() m_chunked = false; utility::string_t name; // check if the client has requested we close the connection - if (m_request.headers().match(header_names::connection, name)) + if (currentRequest.headers().match(header_names::connection, name)) { m_close = boost::iequals(name, U("close")); } - if (m_request.headers().match(header_names::transfer_encoding, name)) + if (currentRequest.headers().match(header_names::transfer_encoding, name)) { m_chunked = boost::ifind_first(name, U("chunked")); } - m_request._get_impl()->_prepare_to_receive_data(); + currentRequest._get_impl()->_prepare_to_receive_data(); if (m_chunked) { ++m_refs; @@ -792,14 +806,14 @@ will_deref_and_erase_t asio_server_connection::handle_headers() return dispatch_request_to_listener(); } - if (!m_request.headers().match(header_names::content_length, m_read_size)) + if (!currentRequest.headers().match(header_names::content_length, m_read_size)) { m_read_size = 0; } if (m_read_size == 0) { - m_request._get_impl()->_complete(0); + currentRequest._get_impl()->_complete(0); } else // need to read the sent data { @@ -816,9 +830,10 @@ will_deref_and_erase_t asio_server_connection::handle_headers() will_deref_t asio_server_connection::handle_chunked_header(const boost::system::error_code& ec) { + auto requestImpl = get_request()._get_impl(); if (ec) { - m_request._get_impl()->_complete(0, std::make_exception_ptr(http_exception(ec.value()))); + requestImpl->_complete(0, std::make_exception_ptr(http_exception(ec.value()))); return deref(); } else @@ -831,7 +846,7 @@ will_deref_t asio_server_connection::handle_chunked_header(const boost::system:: m_read += len; if (len == 0) { - m_request._get_impl()->_complete(m_read); + requestImpl->_complete(m_read); return deref(); } else @@ -847,14 +862,15 @@ will_deref_t asio_server_connection::handle_chunked_header(const boost::system:: will_deref_t asio_server_connection::handle_chunked_body(const boost::system::error_code& ec, int toWrite) { + auto requestImpl = get_request()._get_impl(); if (ec) { - m_request._get_impl()->_complete(0, std::make_exception_ptr(http_exception(ec.value()))); + requestImpl->_complete(0, std::make_exception_ptr(http_exception(ec.value()))); return deref(); } else { - auto writebuf = m_request._get_impl()->outstream().streambuf(); + auto writebuf = requestImpl->outstream().streambuf(); writebuf.putn_nocopy(buffer_cast(m_request_buf.data()), toWrite).then([=](pplx::task writeChunkTask) -> will_deref_t { try @@ -863,7 +879,7 @@ will_deref_t asio_server_connection::handle_chunked_body(const boost::system::er } catch (...) { - m_request._get_impl()->_complete(0, std::current_exception()); + requestImpl->_complete(0, std::current_exception()); return deref(); } @@ -876,16 +892,17 @@ will_deref_t asio_server_connection::handle_chunked_body(const boost::system::er will_deref_t asio_server_connection::handle_body(const boost::system::error_code& ec) { + auto requestImpl = get_request()._get_impl(); // read body if (ec) { - m_request._get_impl()->_complete(0, std::make_exception_ptr(http_exception(ec.value()))); + requestImpl->_complete(0, std::make_exception_ptr(http_exception(ec.value()))); return deref(); } else if (m_read < m_read_size) // there is more to read { - auto writebuf = m_request._get_impl()->outstream().streambuf(); - writebuf.putn_nocopy(boost::asio::buffer_cast(m_request_buf.data()), std::min(m_request_buf.size(), m_read_size - m_read)).then([=](pplx::task writtenSizeTask) -> will_deref_t + auto writebuf = requestImpl->outstream().streambuf(); + writebuf.putn_nocopy(boost::asio::buffer_cast(m_request_buf.data()), std::min(m_request_buf.size(), m_read_size - m_read)).then([this](pplx::task writtenSizeTask) -> will_deref_t { size_t writtenSize = 0; try @@ -894,7 +911,7 @@ will_deref_t asio_server_connection::handle_body(const boost::system::error_code } catch (...) { - m_request._get_impl()->_complete(0, std::current_exception()); + get_request()._get_impl()->_complete(0, std::current_exception()); return deref(); } m_read += writtenSize; @@ -910,7 +927,7 @@ will_deref_t asio_server_connection::handle_body(const boost::system::error_code } else // have read request body { - m_request._get_impl()->_complete(m_read); + requestImpl->_complete(m_read); return deref(); } } @@ -981,14 +998,14 @@ will_deref_and_erase_t asio_server_connection::dispatch_request_to_listener() { // locate the listener: http_listener_impl* pListener = nullptr; - + auto currentRequest = get_request(); try { - pListener = m_p_parent->find_listener(m_request.relative_uri()); + pListener = m_p_parent->find_listener(currentRequest.relative_uri()); } catch (const std::exception&) // may be web::uri_exception, or std::range_error indicating invalid Unicode { - m_request.reply(status_codes::BadRequest); + currentRequest.reply(status_codes::BadRequest); (will_erase_from_parent_t)do_response(); (will_deref_t)deref(); return will_deref_and_erase_t{}; @@ -996,13 +1013,13 @@ will_deref_and_erase_t asio_server_connection::dispatch_request_to_listener() if (pListener == nullptr) { - m_request.reply(status_codes::NotFound); + currentRequest.reply(status_codes::NotFound); (will_erase_from_parent_t)do_response(); (will_deref_t)deref(); return will_deref_and_erase_t{}; } - m_request._set_listener_path(pListener->uri().path()); + currentRequest._set_listener_path(pListener->uri().path()); (will_erase_from_parent_t)do_response(); // Look up the lock for the http_listener. @@ -1013,7 +1030,7 @@ will_deref_and_erase_t asio_server_connection::dispatch_request_to_listener() // It is possible the listener could have unregistered. if(m_p_server->m_registered_listeners.find(pListener) == m_p_server->m_registered_listeners.end()) { - m_request.reply(status_codes::NotFound); + currentRequest.reply(status_codes::NotFound); (will_deref_t)deref(); return will_deref_and_erase_t{}; @@ -1027,13 +1044,13 @@ will_deref_and_erase_t asio_server_connection::dispatch_request_to_listener() try { - pListener->handle_request(m_request); + pListener->handle_request(currentRequest); pListenerLock->unlock(); } catch(...) { pListenerLock->unlock(); - m_request._reply_if_not_already(status_codes::InternalError); + currentRequest._reply_if_not_already(status_codes::InternalError); } (will_deref_t)deref(); From 158752e49bd6c3732a6791445a569736c75c5728 Mon Sep 17 00:00:00 2001 From: Billy Robert O'Neal III Date: Wed, 17 Oct 2018 20:51:26 -0700 Subject: [PATCH 5/5] Avoid VS2015 warning C4800. --- .../tests/functional/http/client/compression_tests.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/Release/tests/functional/http/client/compression_tests.cpp b/Release/tests/functional/http/client/compression_tests.cpp index 889ab71797..8823e2a448 100644 --- a/Release/tests/functional/http/client/compression_tests.cpp +++ b/Release/tests/functional/http/client/compression_tests.cpp @@ -642,7 +642,7 @@ SUITE(compression_tests) // Null decompressor - effectively forces no compression algorithms dv.push_back(std::shared_ptr()); builtin = web::http::compression::details::build_supported_header(ctype, dv); - VERIFY_ARE_EQUAL((bool)transfer, builtin.empty()); + VERIFY_ARE_EQUAL(transfer != 0, builtin.empty()); dv.pop_back(); if (web::http::compression::builtin::supported()) @@ -693,17 +693,17 @@ SUITE(compression_tests) if (c) { VERIFY_IS_TRUE(web::http::compression::builtin::supported()); - VERIFY_IS_FALSE((bool)fake); + VERIFY_IS_FALSE(fake != 0); VERIFY_ARE_EQUAL(c->algorithm(), gzip); } else { - VERIFY_IS_TRUE((bool)fake || !web::http::compression::builtin::supported()); + VERIFY_IS_TRUE(fake != 0 || !web::http::compression::builtin::supported()); } // Supplied compressor - both matching and non-matching c = web::http::compression::details::get_compressor_from_header(*te, ctype, fcv); - VERIFY_ARE_EQUAL((bool)c, (bool)fake); + VERIFY_ARE_EQUAL(c != 0, fake != 0); if (c) { VERIFY_ARE_EQUAL(c->algorithm(), fake_provider::FAKE);