From 01720ce82bbec9992537bce58c2decc23c1b79b5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?G=C3=BCnter=20Obiltschnig?= Date: Tue, 15 Jun 2021 13:30:51 +0200 Subject: [PATCH] #3019: ObjectPool wait on borrow condition fix --- Foundation/include/Poco/ObjectPool.h | 65 +++++++++++---------- Foundation/testsuite/src/ObjectPoolTest.cpp | 43 ++++++++++---- Foundation/testsuite/src/ObjectPoolTest.h | 1 + 3 files changed, 68 insertions(+), 41 deletions(-) diff --git a/Foundation/include/Poco/ObjectPool.h b/Foundation/include/Poco/ObjectPool.h index 3933e13b3..ab86b15e8 100644 --- a/Foundation/include/Poco/ObjectPool.h +++ b/Foundation/include/Poco/ObjectPool.h @@ -47,7 +47,7 @@ public: { return new C; } - + bool validateObject(P pObject) /// Checks whether the object is still valid /// and can be reused. @@ -60,14 +60,14 @@ public: { return true; } - + void activateObject(P pObject) /// Called before an object is handed out by the pool. /// Also called for newly created objects, before /// they are given out for the first time. { } - + void deactivateObject(P pObject) /// Called after an object has been given back to the /// pool and the object is still valid (a prior call @@ -77,7 +77,7 @@ public: /// must not throw an exception. { } - + void destroyObject(P pObject) /// Destroy an object. /// @@ -97,20 +97,20 @@ public: { return new C; } - + bool validateObject(Poco::AutoPtr pObject) { return true; } - + void activateObject(Poco::AutoPtr pObject) { } - + void deactivateObject(Poco::AutoPtr pObject) { } - + void destroyObject(Poco::AutoPtr pObject) { } @@ -125,20 +125,20 @@ public: { return new C; } - + bool validateObject(Poco::SharedPtr pObject) { return true; } - + void activateObject(Poco::SharedPtr pObject) { } - + void deactivateObject(Poco::SharedPtr pObject) { } - + void destroyObject(Poco::SharedPtr pObject) { } @@ -177,7 +177,7 @@ public: { poco_assert (capacity <= peakCapacity); } - + ObjectPool(const F& factory, std::size_t capacity, std::size_t peakCapacity): /// Creates a new ObjectPool with the given PoolableObjectFactory, /// capacity and peak capacity. The PoolableObjectFactory must have @@ -189,7 +189,7 @@ public: { poco_assert (capacity <= peakCapacity); } - + ~ObjectPool() /// Destroys the ObjectPool. { @@ -205,7 +205,7 @@ public: poco_unexpected(); } } - + P borrowObject(long timeoutMilliseconds = 0) /// Obtains an object from the pool, or creates a new object if /// possible. @@ -217,22 +217,15 @@ public: { Poco::FastMutex::ScopedLock lock(_mutex); - if (!_pool.empty()) - { - P pObject = _pool.back(); - _pool.pop_back(); - return activateObject(pObject); - } - - if (_size >= _peakCapacity) + if (_size >= _peakCapacity && _pool.empty()) { if (timeoutMilliseconds == 0) { return 0; } - while (_size >= _peakCapacity) + while (_size >= _peakCapacity && _pool.empty()) { - if ( !_availableCondition.tryWait(_mutex, timeoutMilliseconds)) + if (!_availableCondition.tryWait(_mutex, timeoutMilliseconds)) { // timeout return 0; @@ -240,10 +233,19 @@ public: } } + if (!_pool.empty()) + { + P pObject = _pool.back(); + _pool.pop_back(); + + return activateObject(pObject); + } + // _size < _peakCapacity P pObject = _factory.createObject(); activateObject(pObject); _size++; + return pObject; } @@ -260,6 +262,7 @@ public: try { _pool.push_back(pObject); + _availableCondition.signal(); return; } catch (...) @@ -276,19 +279,19 @@ public: { return _capacity; } - + std::size_t peakCapacity() const { return _peakCapacity; } - + std::size_t size() const { Poco::FastMutex::ScopedLock lock(_mutex); - + return _size; } - + std::size_t available() const { Poco::FastMutex::ScopedLock lock(_mutex); @@ -310,12 +313,12 @@ protected: } return pObject; } - + private: ObjectPool(); ObjectPool(const ObjectPool&); ObjectPool& operator = (const ObjectPool&); - + F _factory; std::size_t _capacity; std::size_t _peakCapacity; diff --git a/Foundation/testsuite/src/ObjectPoolTest.cpp b/Foundation/testsuite/src/ObjectPoolTest.cpp index f4d9fd791..d29bfbc50 100644 --- a/Foundation/testsuite/src/ObjectPoolTest.cpp +++ b/Foundation/testsuite/src/ObjectPoolTest.cpp @@ -13,6 +13,7 @@ #include "CppUnit/TestSuite.h" #include "Poco/ObjectPool.h" #include "Poco/Exception.h" +#include "Poco/Thread.h" using Poco::ObjectPool; @@ -30,18 +31,18 @@ ObjectPoolTest::~ObjectPoolTest() void ObjectPoolTest::testObjectPool() { - ObjectPool > pool(3, 4); - + ObjectPool> pool(3, 4); + assertTrue (pool.capacity() == 3); assertTrue (pool.peakCapacity() == 4); assertTrue (pool.size() == 0); assertTrue (pool.available() == 4); - + Poco::SharedPtr pStr1 = pool.borrowObject(); pStr1->assign("first"); assertTrue (pool.size() == 1); assertTrue (pool.available() == 3); - + Poco::SharedPtr pStr2 = pool.borrowObject(); pStr2->assign("second"); assertTrue (pool.size() == 2); @@ -51,19 +52,19 @@ void ObjectPoolTest::testObjectPool() pStr3->assign("third"); assertTrue (pool.size() == 3); assertTrue (pool.available() == 1); - + Poco::SharedPtr pStr4 = pool.borrowObject(); pStr4->assign("fourth"); assertTrue (pool.size() == 4); assertTrue (pool.available() == 0); - + Poco::SharedPtr pStr5 = pool.borrowObject(); assertTrue (pStr5.isNull()); - + pool.returnObject(pStr4); assertTrue (pool.size() == 4); assertTrue (pool.available() == 1); - + pool.returnObject(pStr3); assertTrue (pool.size() == 4); assertTrue (pool.available() == 2); @@ -75,10 +76,10 @@ void ObjectPoolTest::testObjectPool() pool.returnObject(pStr3); pool.returnObject(pStr2); pool.returnObject(pStr1); - + assertTrue (pool.size() == 3); assertTrue (pool.available() == 4); - + pStr1 = pool.borrowObject(); assertTrue (*pStr1 == "second"); assertTrue (pool.available() == 3); @@ -88,6 +89,27 @@ void ObjectPoolTest::testObjectPool() } +void ObjectPoolTest::testObjectPoolWaitOnBorrowObject() +{ + ObjectPool> pool(1, 1); + + Poco::SharedPtr objectToReturnDuringBorrow = pool.borrowObject(); + + Poco::Thread threadToReturnObject; + threadToReturnObject.startFunc( + [&pool, &objectToReturnDuringBorrow]() + { + pool.returnObject(objectToReturnDuringBorrow); + } + ); + + Poco::SharedPtr object = pool.borrowObject(1000); + + threadToReturnObject.join(); + assertFalse(object.isNull()); +} + + void ObjectPoolTest::setUp() { } @@ -103,6 +125,7 @@ CppUnit::Test* ObjectPoolTest::suite() CppUnit::TestSuite* pSuite = new CppUnit::TestSuite("ObjectPoolTest"); CppUnit_addTest(pSuite, ObjectPoolTest, testObjectPool); + CppUnit_addTest(pSuite, ObjectPoolTest, testObjectPoolWaitOnBorrowObject); return pSuite; } diff --git a/Foundation/testsuite/src/ObjectPoolTest.h b/Foundation/testsuite/src/ObjectPoolTest.h index 4ff31d190..16131eb17 100644 --- a/Foundation/testsuite/src/ObjectPoolTest.h +++ b/Foundation/testsuite/src/ObjectPoolTest.h @@ -25,6 +25,7 @@ public: ~ObjectPoolTest(); void testObjectPool(); + void testObjectPoolWaitOnBorrowObject(); void setUp(); void tearDown();