From a7840d49647faf8c61db8941fdcfc7a366522ca8 Mon Sep 17 00:00:00 2001 From: Soemin Tjong Date: Wed, 31 May 2017 18:59:43 -0700 Subject: [PATCH] Fix potential infinite wait in IPCAFactoryReset() and IPCAReboot(). Without the predicate parameter, the wait_for() won't be unblocked from earlier notify_all(). Also, relax the timing waiting for notifications in the unit tests. Change-Id: I04d34f8221dc92239b27877b365dcb0d211c3458 Signed-off-by: Soemin Tjong Reviewed-on: https://gerrit.iotivity.org/gerrit/20519 Tested-by: jenkins-iotivity Reviewed-by: Dan Mihai --- resource/IPCA/src/ipca.cpp | 14 ++++++++++++-- resource/IPCA/unittests/ipcaunittests.cpp | 3 ++- 2 files changed, 14 insertions(+), 3 deletions(-) diff --git a/resource/IPCA/src/ipca.cpp b/resource/IPCA/src/ipca.cpp index 28d3d0c..c5f2a7f 100644 --- a/resource/IPCA/src/ipca.cpp +++ b/resource/IPCA/src/ipca.cpp @@ -308,6 +308,7 @@ typedef struct std::mutex syncMutex; std::condition_variable condVar; IPCAStatus result; + bool callbackComplete; } AsyncContext; void IPCA_CALL AsyncCallback( @@ -318,6 +319,7 @@ void IPCA_CALL AsyncCallback( OC_UNUSED(propertyBagHandle); AsyncContext* async = reinterpret_cast(context); + async->callbackComplete = true; async->result = result; async->condVar.notify_all(); } @@ -327,6 +329,7 @@ IPCAStatus IPCA_CALL IPCAFactoryReset(IPCADeviceHandle deviceHandle) IPCAStatus status; AsyncContext factoryResetContext; + factoryResetContext.callbackComplete = false; std::unique_lock lock { factoryResetContext.syncMutex }; IPCAPropertyBagHandle propertyBagHandle; @@ -356,7 +359,10 @@ IPCAStatus IPCA_CALL IPCAFactoryReset(IPCADeviceHandle deviceHandle) if (status == IPCA_OK) { - factoryResetContext.condVar.wait_for(lock, std::chrono::milliseconds{ INT_MAX }); + factoryResetContext.condVar.wait_for( + lock, + std::chrono::milliseconds{ INT_MAX }, + [&]{return factoryResetContext.callbackComplete;}); IPCAPropertyBagDestroy(propertyBagHandle); return factoryResetContext.result; } @@ -370,6 +376,7 @@ IPCAStatus IPCA_CALL IPCAReboot(IPCADeviceHandle deviceHandle) IPCAStatus status; AsyncContext rebootContext; + rebootContext.callbackComplete = false; std::unique_lock lock { rebootContext.syncMutex }; IPCAPropertyBagHandle propertyBagHandle; @@ -399,7 +406,10 @@ IPCAStatus IPCA_CALL IPCAReboot(IPCADeviceHandle deviceHandle) if (status == IPCA_OK) { - rebootContext.condVar.wait_for(lock, std::chrono::milliseconds{ INT_MAX }); + rebootContext.condVar.wait_for( + lock, + std::chrono::milliseconds{ INT_MAX }, + [&]{return rebootContext.callbackComplete;}); IPCAPropertyBagDestroy(propertyBagHandle); return rebootContext.result; } diff --git a/resource/IPCA/unittests/ipcaunittests.cpp b/resource/IPCA/unittests/ipcaunittests.cpp index cbca314..f15879a 100644 --- a/resource/IPCA/unittests/ipcaunittests.cpp +++ b/resource/IPCA/unittests/ipcaunittests.cpp @@ -586,10 +586,11 @@ TEST_F(IPCAElevatorClient, SuccessfullyReceiveResourceChangeNotifications) ASSERT_EQ(true, result); // Wait until observed current floor is set to targeted floor. + // Outstanding requests should time out in 247 seconds (EXCHANGE_LIFETIME) per rfc 7252. std::unique_lock lock(m_resourceChangeCbMutex); m_resourceChangeCbCV.wait_for( lock, - std::chrono::seconds(10), + std::chrono::seconds(247), [this] { return GetObservedCurrentFloor() == 10; }); EXPECT_EQ(10, GetObservedCurrentFloor()); // check it is at floor 10. -- 2.7.4