From a60c98bb02391f995a03c3db5898e146ff2f6f3b Mon Sep 17 00:00:00 2001 From: Brian Aker Date: Sun, 25 Sep 2011 23:13:04 -0700 Subject: [PATCH] Extend pool API for fetch/release. Fix concurrency issue in test case. --- docs/conf.py.in | 3 +- docs/include.am | 3 + docs/memcached_pool.rst | 35 +++-- libmemcached/error.cc | 6 +- libmemcached/util/pool.cc | 281 +++++++++++++++++++++-------------- libmemcached/util/pool.h | 5 + support/libmemcached.spec.in | 3 + tests/mem_functions.cc | 228 +++++++++++++++++----------- 8 files changed, 352 insertions(+), 212 deletions(-) diff --git a/docs/conf.py.in b/docs/conf.py.in index c485d77c..5952dc2e 100644 --- a/docs/conf.py.in +++ b/docs/conf.py.in @@ -290,9 +290,10 @@ man_pages = [ ('memcached_pool', 'memcached_pool_behavior_set', u'libmemcached Documentation', [u'Brian Aker'], 3), ('memcached_pool', 'memcached_pool_create', u'libmemcached Documentation', [u'Brian Aker'], 3), ('memcached_pool', 'memcached_pool_destroy', u'libmemcached Documentation', [u'Brian Aker'], 3), + ('memcached_pool', 'memcached_pool_fetch', u'libmemcached Documentation', [u'Brian Aker'], 3), ('memcached_pool', 'memcached_pool_pop', u'libmemcached Documentation', [u'Brian Aker'], 3), ('memcached_pool', 'memcached_pool_push', u'libmemcached Documentation', [u'Brian Aker'], 3), - ('memcached_pool', 'memcached_pool_push', u'libmemcached Documentation', [u'Brian Aker'], 3), + ('memcached_pool', 'memcached_pool_release', u'libmemcached Documentation', [u'Brian Aker'], 3), ('memcached_pool', 'memcached_pool_st', u'libmemcached Documentation', [u'Brian Aker'], 3), ('memcached_quit', 'memcached_quit', u'libmemcached Documentation', [u'Brian Aker'], 3), ('memcached_result_st', 'memcached_result_cas', u'Working with result sets', [u'Brian Aker'], 3), diff --git a/docs/include.am b/docs/include.am index 1a1feac0..51b9ed73 100644 --- a/docs/include.am +++ b/docs/include.am @@ -80,12 +80,15 @@ man_MANS+= \ docs/man/memcached_mget_by_key.3 \ docs/man/memcached_mget_execute.3 \ docs/man/memcached_mget_execute_by_key.3 \ + docs/man/memcached_pool.3 \ docs/man/memcached_pool_behavior_get.3 \ docs/man/memcached_pool_behavior_set.3 \ docs/man/memcached_pool_create.3 \ docs/man/memcached_pool_destroy.3 \ + docs/man/memcached_pool_fetch.3 \ docs/man/memcached_pool_pop.3 \ docs/man/memcached_pool_push.3 \ + docs/man/memcached_pool_release.3 \ docs/man/memcached_pool_st.3 \ docs/man/memcached_prepend.3 \ docs/man/memcached_prepend_by_key.3 \ diff --git a/docs/memcached_pool.rst b/docs/memcached_pool.rst index 7dcc707a..3f8882f8 100644 --- a/docs/memcached_pool.rst +++ b/docs/memcached_pool.rst @@ -14,13 +14,26 @@ SYNOPSIS .. c:function:: memcached_pool_st* memcached_pool_create(memcached_st* mmc, int initial, int max) .. deprecated:: 0.46 - Use :c:func:`memcached_pool()` instead. + Use :c:func:`memcached_pool()` .. c:function:: memcached_st* memcached_pool_destroy(memcached_pool_st* pool) -.. c:function:: memcached_st* memcached_pool_pop (memcached_pool_st* pool, bool block, memcached_return_t *rc) - +.. c:function:: memcached_st* memcached_pool_pop(memcached_pool_st* pool, bool block, memcached_return_t *rc) +.. deprecated:: 0.53 + Use :c:func:`memcached_pool_fetch()` + +.. c:function:: memcached_st* memcached_pool_fetch(memcached_pool_st*, struct timespec* relative_time, memcached_return_t* rc); + +.. versionadded:: 0.53 + Synonym for memcached_pool_pop() + .. c:function:: memcached_return_t memcached_pool_push(memcached_pool_st* pool, memcached_st *mmc) +.. deprecated:: 0.53 + Use :c:func:`memcached_pool_push()` + +.. c:function:: memcached_return_t memcached_pool_release(memcached_pool_st* pool, memcached_st* mmc); +.. versionadded:: 0.53 + Synonym for memcached_pool_push(). .. c:function:: memcached_return_t memcached_pool_behavior_set(memcached_pool_st *pool, memcached_behavior_t flag, uint64_t data) @@ -28,7 +41,6 @@ SYNOPSIS Compile and link with -lmemcachedutil -lmemcached - ----------- DESCRIPTION ----------- @@ -43,15 +55,16 @@ created with :c:func:`memcached_pool_create()` and release all allocated resources. It will return the pointer to the :c:type:`memcached_st` structure passed as an argument to :c:func:`memcached_pool_create()`, and returns the ownership of the pointer to the caller when created with :c:func:`memcached_pool_create()`, otherwise NULL is returned.. -:c:func:`memcached_pool_pop()` is used to grab a connection structure from the -connection pool. The block argument specifies if the function should +:c:func:`memcached_pool_fetch()` is used to fetch a connection structure from the +connection pool. The relative_time argument specifies if the function should block and wait for a connection structure to be available if we try -to exceed the maximum size. +to exceed the maximum size. You need to specify time in relative time. -:c:func:`memcached_pool_push()` is used to return a connection structure back to the pool. +:c:func:`memcached_pool_release()` is used to return a connection structure back to the pool. :c:func:`memcached_pool_behavior_get()` and :c:func:`memcached_pool_behavior_set()` is used to get/set behavior flags on all connections in the pool. +Both :c:func:`memcached_pool_release()` and :c:func:`memcached_pool_fetch()` are thread safe. ------ RETURN @@ -61,10 +74,14 @@ RETURN :c:func:`memcached_pool_pop()` returns a pointer to a :c:type:`memcached_st` structure from the pool (or NULL if an allocation cannot be satisfied). -:c:func:`memcached_pool_push()` returns :c:type:`MEMCACHED_SUCCESS` upon success. +:c:func:`memcached_pool_release()` returns :c:type:`MEMCACHED_SUCCESS` upon success. :c:func:`memcached_pool_behavior_get()` and :c:func:`memcached_pool_behavior_get()` returns :c:type:`MEMCACHED_SUCCESS` upon success. +If any methods returns MEMCACHED_IN_PROGRESS then a lock on the pool could not be obtained. If any of the parameters passed to any of these functions is invalid, MEMCACHED_INVALID_ARGUMENTS will be returned. + +memcached_pool_fetch() may return MEMCACHED_TIMEOUT if a timeout occurs while waiting for a free memcached_st. MEMCACHED_NOTFOUND if no memcached_st was available. + ---- HOME diff --git a/libmemcached/error.cc b/libmemcached/error.cc index afb60645..d6b7f166 100644 --- a/libmemcached/error.cc +++ b/libmemcached/error.cc @@ -361,10 +361,12 @@ memcached_return_t memcached_set_errno(memcached_server_st& self, int local_errn static void _error_print(const memcached_error_t *error) { - if (not error) + if (error == NULL) + { return; + } - if (not error->size) + if (error->size == 0) { fprintf(stderr, "%s\n", memcached_strerror(NULL, error->rc) ); } diff --git a/libmemcached/util/pool.cc b/libmemcached/util/pool.cc index e89789d4..47f46eec 100644 --- a/libmemcached/util/pool.cc +++ b/libmemcached/util/pool.cc @@ -39,13 +39,13 @@ #include #include +#include + #include #include #include #include -static bool grow_pool(memcached_pool_st* pool); - struct memcached_pool_st { pthread_mutex_t mutex; @@ -56,6 +56,7 @@ struct memcached_pool_st const uint32_t size; uint32_t current_size; bool _owns_master; + struct timespec _timeout; memcached_pool_st(memcached_st *master_arg, size_t max_arg) : master(master_arg), @@ -67,26 +68,21 @@ struct memcached_pool_st { pthread_mutex_init(&mutex, NULL); pthread_cond_init(&cond, NULL); + _timeout.tv_sec= 5; + _timeout.tv_nsec= 0; } - bool init(uint32_t initial) + const struct timespec& timeout() const { - server_pool= new (std::nothrow) memcached_st *[size]; - if (not server_pool) - return false; + return _timeout; + } - /* - Try to create the initial size of the pool. An allocation failure at - this time is not fatal.. - */ - for (unsigned int x= 0; x < initial; ++x) - { - if (not grow_pool(this)) - break; - } + bool release(memcached_st*, memcached_return_t& rc); - return true; - } + memcached_st *fetch(memcached_return_t& rc); + memcached_st *fetch(const struct timespec&, memcached_return_t& rc); + + bool init(uint32_t initial); ~memcached_pool_st() { @@ -121,27 +117,6 @@ struct memcached_pool_st } }; -static memcached_return_t mutex_enter(pthread_mutex_t *mutex) -{ - int ret; - do - { - ret= pthread_mutex_lock(mutex); - } while (ret == -1 && errno == EINTR); - - return (ret == -1) ? MEMCACHED_ERRNO : MEMCACHED_SUCCESS; -} - -static memcached_return_t mutex_exit(pthread_mutex_t *mutex) -{ - int ret; - do - { - ret= pthread_mutex_unlock(mutex); - } while (ret == -1 && errno == EINTR); - - return (ret == -1) ? MEMCACHED_ERRNO : MEMCACHED_SUCCESS; -} /** * Grow the connection pool by creating a connection structure and clone the @@ -149,6 +124,8 @@ static memcached_return_t mutex_exit(pthread_mutex_t *mutex) */ static bool grow_pool(memcached_pool_st* pool) { + assert(pool); + memcached_st *obj; if (not (obj= memcached_clone(NULL, pool->master))) { @@ -162,18 +139,38 @@ static bool grow_pool(memcached_pool_st* pool) return true; } +bool memcached_pool_st::init(uint32_t initial) +{ + server_pool= new (std::nothrow) memcached_st *[size]; + if (not server_pool) + return false; + + /* + Try to create the initial size of the pool. An allocation failure at + this time is not fatal.. + */ + for (unsigned int x= 0; x < initial; ++x) + { + if (grow_pool(this) == false) + { + break; + } + } + + return true; +} + + static inline memcached_pool_st *_pool_create(memcached_st* master, uint32_t initial, uint32_t max) { if (initial == 0 or max == 0 or (initial > max)) { - errno= EINVAL; return NULL; } memcached_pool_st *object= new (std::nothrow) memcached_pool_st(master, max); if (object == NULL) { - errno= ENOMEM; // Set this for the failed calloc return NULL; } @@ -208,10 +205,8 @@ memcached_pool_st * memcached_pool(const char *option_string, size_t option_stri if (self == NULL) { memcached_free(memc); - errno= ENOMEM; return NULL; } - errno= 0; self->_owns_master= true; @@ -239,121 +234,180 @@ memcached_st* memcached_pool_destroy(memcached_pool_st* pool) return ret; } -memcached_st* memcached_pool_pop(memcached_pool_st* pool, - bool block, - memcached_return_t *rc) +memcached_st* memcached_pool_st::fetch(memcached_return_t& rc) { - memcached_return_t not_used; - if (rc == NULL) - { - rc= ¬_used; - } - - if (pool == NULL) - { - *rc= MEMCACHED_INVALID_ARGUMENTS; - errno= EINVAL; + static struct timespec relative_time= { 0, 0 }; + return fetch(relative_time, rc); +} - return NULL; - } +memcached_st* memcached_pool_st::fetch(const struct timespec& relative_time, memcached_return_t& rc) +{ + rc= MEMCACHED_SUCCESS; - if (memcached_failed((*rc= mutex_enter(&pool->mutex)))) + if (pthread_mutex_lock(&mutex)) { + rc= MEMCACHED_IN_PROGRESS; return NULL; } memcached_st *ret= NULL; do { - if (pool->firstfree > -1) + if (firstfree > -1) { - ret= pool->server_pool[pool->firstfree--]; + ret= server_pool[firstfree--]; } - else if (pool->current_size == pool->size) + else if (current_size == size) { - if (block == false) + if (relative_time.tv_sec == 0 and relative_time.tv_nsec == 0) { - *rc= mutex_exit(&pool->mutex); // this should be a different error + pthread_mutex_unlock(&mutex); + rc= MEMCACHED_NOTFOUND; + return NULL; } struct timespec time_to_wait= {0, 0}; - time_to_wait.tv_sec = time(NULL) +5; + time_to_wait.tv_sec= time(NULL) +relative_time.tv_sec; + time_to_wait.tv_nsec= relative_time.tv_nsec; + int thread_ret; - if ((thread_ret= pthread_cond_timedwait(&pool->cond, &pool->mutex, &time_to_wait)) != 0) + if ((thread_ret= pthread_cond_timedwait(&cond, &mutex, &time_to_wait)) != 0) { - mutex_exit(&pool->mutex); - errno= thread_ret; + pthread_mutex_unlock(&mutex); if (thread_ret == ETIMEDOUT) { - *rc= MEMCACHED_TIMEOUT; + rc= MEMCACHED_TIMEOUT; } else { - *rc= MEMCACHED_ERRNO; + errno= thread_ret; + rc= MEMCACHED_ERRNO; } return NULL; } } - else if (not grow_pool(pool)) + else if (grow_pool(this) == false) { - (void)mutex_exit(&pool->mutex); - *rc= MEMCACHED_MEMORY_ALLOCATION_FAILURE; + (void)pthread_mutex_unlock(&mutex); return NULL; } } while (ret == NULL); - *rc= mutex_exit(&pool->mutex); + pthread_mutex_unlock(&mutex); return ret; } -memcached_return_t memcached_pool_push(memcached_pool_st* pool, memcached_st *released) +bool memcached_pool_st::release(memcached_st *released, memcached_return_t& rc) { - if (pool == NULL) - { - return MEMCACHED_INVALID_ARGUMENTS; - } - + rc= MEMCACHED_SUCCESS; if (released == NULL) { - return MEMCACHED_INVALID_ARGUMENTS; + rc= MEMCACHED_INVALID_ARGUMENTS; + return false; } - memcached_return_t rc= mutex_enter(&pool->mutex); - - if (rc != MEMCACHED_SUCCESS) + if (pthread_mutex_lock(&mutex)) { - return rc; + rc= MEMCACHED_IN_PROGRESS; + return false; } - /* Someone updated the behavior on the object.. */ - if (pool->compare_version(released) == false) + /* + Someone updated the behavior on the object, so we clone a new memcached_st with the new settings. If we fail to clone, we keep the old one around. + */ + if (compare_version(released) == false) { - memcached_free(released); - if (not (released= memcached_clone(NULL, pool->master))) + memcached_st *memc; + if ((memc= memcached_clone(NULL, master))) { - rc= MEMCACHED_SOME_ERRORS; + memcached_free(released); + released= memc; } } - pool->server_pool[++pool->firstfree]= released; + server_pool[++firstfree]= released; - if (pool->firstfree == 0 and pool->current_size == pool->size) + if (firstfree == 0 and current_size == size) { /* we might have people waiting for a connection.. wake them up :-) */ - pthread_cond_broadcast(&pool->cond); + pthread_cond_broadcast(&cond); } - memcached_return_t rval= mutex_exit(&pool->mutex); - if (rc == MEMCACHED_SOME_ERRORS) + (void)pthread_mutex_unlock(&mutex); + + return true; +} + +memcached_st* memcached_pool_fetch(memcached_pool_st* pool, struct timespec* relative_time, memcached_return_t* rc) +{ + if (pool == NULL) { - return rc; + return NULL; + } + + memcached_return_t unused; + if (rc == NULL) + { + rc= &unused; } - return rval; + if (relative_time == NULL) + { + return pool->fetch(*rc); + } + + return pool->fetch(*relative_time, *rc); +} + +memcached_st* memcached_pool_pop(memcached_pool_st* pool, + bool block, + memcached_return_t *rc) +{ + if (pool == NULL) + { + return NULL; + } + + memcached_return_t unused; + if (rc == NULL) + { + rc= &unused; + } + + memcached_st *memc; + if (block) + { + memc= pool->fetch(pool->timeout(), *rc); + } + else + { + memc= pool->fetch(*rc); + } + + return memc; +} + +memcached_return_t memcached_pool_release(memcached_pool_st* pool, memcached_st *released) +{ + if (pool == NULL) + { + return MEMCACHED_INVALID_ARGUMENTS; + } + + memcached_return_t rc; + + (void) pool->release(released, rc); + + return rc; +} + +memcached_return_t memcached_pool_push(memcached_pool_st* pool, memcached_st *released) +{ + return memcached_pool_release(pool, released); } @@ -366,15 +420,16 @@ memcached_return_t memcached_pool_behavior_set(memcached_pool_st *pool, return MEMCACHED_INVALID_ARGUMENTS; } - memcached_return_t rc= mutex_enter(&pool->mutex); - if (rc != MEMCACHED_SUCCESS) - return rc; + if (pthread_mutex_lock(&pool->mutex)) + { + return MEMCACHED_IN_PROGRESS; + } /* update the master */ - rc= memcached_behavior_set(pool->master, flag, data); - if (rc != MEMCACHED_SUCCESS) + memcached_return_t rc= memcached_behavior_set(pool->master, flag, data); + if (memcached_failed(rc)) { - mutex_exit(&pool->mutex); + (void)pthread_mutex_unlock(&pool->mutex); return rc; } @@ -382,16 +437,17 @@ memcached_return_t memcached_pool_behavior_set(memcached_pool_st *pool, /* update the clones */ for (int xx= 0; xx <= pool->firstfree; ++xx) { - rc= memcached_behavior_set(pool->server_pool[xx], flag, data); - if (rc == MEMCACHED_SUCCESS) + if (memcached_success(memcached_behavior_set(pool->server_pool[xx], flag, data))) { pool->server_pool[xx]->configure.version= pool->version(); } else { - memcached_free(pool->server_pool[xx]); - if (not (pool->server_pool[xx]= memcached_clone(NULL, pool->master))) + memcached_st *memc; + if ((memc= memcached_clone(NULL, pool->master))) { + memcached_free(pool->server_pool[xx]); + pool->server_pool[xx]= memc; /* I'm not sure what to do in this case.. this would happen if we fail to push the server list inside the client.. I should add a testcase for this, but I believe the following @@ -402,7 +458,9 @@ memcached_return_t memcached_pool_behavior_set(memcached_pool_st *pool, } } - return mutex_exit(&pool->mutex); + (void)pthread_mutex_unlock(&pool->mutex); + + return rc; } memcached_return_t memcached_pool_behavior_get(memcached_pool_st *pool, @@ -414,13 +472,14 @@ memcached_return_t memcached_pool_behavior_get(memcached_pool_st *pool, return MEMCACHED_INVALID_ARGUMENTS; } - memcached_return_t rc= mutex_enter(&pool->mutex); - if (rc != MEMCACHED_SUCCESS) + if (pthread_mutex_lock(&pool->mutex)) { - return rc; + return MEMCACHED_IN_PROGRESS; } *value= memcached_behavior_get(pool->master, flag); - return mutex_exit(&pool->mutex); + (void)pthread_mutex_unlock(&pool->mutex); + + return MEMCACHED_SUCCESS; } diff --git a/libmemcached/util/pool.h b/libmemcached/util/pool.h index e4a6ce59..eba97ec2 100644 --- a/libmemcached/util/pool.h +++ b/libmemcached/util/pool.h @@ -63,6 +63,11 @@ memcached_st* memcached_pool_pop(memcached_pool_st* pool, LIBMEMCACHED_API memcached_return_t memcached_pool_push(memcached_pool_st* pool, memcached_st* mmc); +LIBMEMCACHED_API + memcached_return_t memcached_pool_release(memcached_pool_st* pool, memcached_st* mmc); + +LIBMEMCACHED_API +memcached_st* memcached_pool_fetch(memcached_pool_st*, struct timespec* relative_time, memcached_return_t* rc); LIBMEMCACHED_API memcached_return_t memcached_pool_behavior_set(memcached_pool_st *ptr, diff --git a/support/libmemcached.spec.in b/support/libmemcached.spec.in index c031b7b3..64abfc32 100644 --- a/support/libmemcached.spec.in +++ b/support/libmemcached.spec.in @@ -245,7 +245,10 @@ you will need to install %{name}-devel. %{_mandir}/man3/memcached_pool_destroy.3.gz %{_mandir}/man3/memcached_pool_pop.3.gz %{_mandir}/man3/memcached_pool_push.3.gz +%{_mandir}/man3/memcached_pool_fetch.3.gz +%{_mandir}/man3/memcached_pool_release.3.gz %{_mandir}/man3/memcached_pool_st.3.gz +%{_mandir}/man3/memcached_pool.3.gz %{_mandir}/man3/memcached_prepend.3.gz %{_mandir}/man3/memcached_prepend_by_key.3.gz %{_mandir}/man3/memcached_quit.3.gz diff --git a/tests/mem_functions.cc b/tests/mem_functions.cc index c6f627f7..cca23464 100644 --- a/tests/mem_functions.cc +++ b/tests/mem_functions.cc @@ -4192,41 +4192,6 @@ static test_return_t dump_test(memcached_st *memc) return TEST_SUCCESS; } -struct test_pool_context_st { - volatile memcached_return_t rc; - memcached_pool_st* pool; - memcached_st* mmc; - sem_t lock; - - test_pool_context_st(memcached_pool_st *pool_arg, memcached_st *memc_arg): - rc(MEMCACHED_FAILURE), - pool(pool_arg), - mmc(memc_arg) - { - sem_init(&lock, 0, 0); - } - - ~test_pool_context_st() - { - sem_destroy(&lock); - } -}; - -static void* connection_release(void *arg) -{ - test_pool_context_st *resource= static_cast(arg); - assert(resource); - if (resource == NULL) - { - abort(); - } - - sem_wait(&resource->lock); - // Release all of the memc we are holding - resource->rc= memcached_pool_push(resource->pool, resource->mmc); - - pthread_exit(arg); -} #define POOL_SIZE 10 static test_return_t connection_pool_test(memcached_st *memc) @@ -4234,55 +4199,21 @@ static test_return_t connection_pool_test(memcached_st *memc) memcached_pool_st* pool= memcached_pool_create(memc, 5, POOL_SIZE); test_true(pool); memcached_st *mmc[POOL_SIZE]; - memcached_return_t rc; // Fill up our array that we will store the memc that are in the pool for (size_t x= 0; x < POOL_SIZE; ++x) { - mmc[x]= memcached_pool_pop(pool, false, &rc); - test_true(mmc[x]); + memcached_return_t rc; + mmc[x]= memcached_pool_fetch(pool, NULL, &rc); test_compare(MEMCACHED_SUCCESS, rc); + test_true(mmc[x]); } // All memc should be gone - test_null(memcached_pool_pop(pool, false, &rc)); - test_compare(MEMCACHED_SUCCESS, rc); - - /* - @note This comment was written to describe what was believed to be the original authors intent. - - This portion of the test creates a thread that will wait until told to free a memcached_st - that will be grabbed by the main thread. - - It is believed that this tests whether or not we are handling ownership correctly. - */ { - pthread_t tid; - test_pool_context_st item(pool, mmc[9]); - - test_zero(pthread_create(&tid, NULL, connection_release, &item)); - mmc[9]= memcached_pool_pop(pool, true, &rc); - if (rc != MEMCACHED_SUCCESS) - { - sem_post(&item.lock); - pthread_join(tid, NULL); - } - else if (rc == MEMCACHED_TIMEOUT) - { - Error << "Possible timing issue with memcached_pool_pop"; - } - else - { - test_compare(MEMCACHED_SUCCESS, rc); - } - - sem_post(&item.lock); - pthread_join(tid, NULL); - if (rc == MEMCACHED_SUCCESS) - { - test_compare(MEMCACHED_SUCCESS, item.rc); - test_true(mmc[9]); - } + memcached_return_t rc; + test_null(memcached_pool_fetch(pool, NULL, &rc)); + test_compare(MEMCACHED_NOTFOUND, rc); } // Release them.. @@ -4290,7 +4221,7 @@ static test_return_t connection_pool_test(memcached_st *memc) { if (mmc[x]) { - test_compare(MEMCACHED_SUCCESS, memcached_pool_push(pool, mmc[x])); + test_compare(MEMCACHED_SUCCESS, memcached_pool_release(pool, mmc[x])); } } test_true(memcached_pool_destroy(pool) == memc); @@ -4303,19 +4234,22 @@ static test_return_t connection_pool2_test(memcached_st *memc) memcached_pool_st* pool= memcached_pool_create(memc, 5, POOL_SIZE); test_true(pool); memcached_st *mmc[POOL_SIZE]; - memcached_return_t rc; // Fill up our array that we will store the memc that are in the pool for (size_t x= 0; x < POOL_SIZE; ++x) { - mmc[x]= memcached_pool_pop(pool, false, &rc); - test_true(mmc[x]); + memcached_return_t rc; + mmc[x]= memcached_pool_fetch(pool, NULL, &rc); test_compare(MEMCACHED_SUCCESS, rc); + test_true(mmc[x]); } // All memc should be gone - test_null(memcached_pool_pop(pool, false, &rc)); - test_compare(MEMCACHED_SUCCESS, rc); + { + memcached_return_t rc; + test_null(memcached_pool_fetch(pool, NULL, &rc)); + test_compare(MEMCACHED_NOTFOUND, rc); + } // verify that I can do ops with all connections test_compare(MEMCACHED_SUCCESS, @@ -4336,7 +4270,7 @@ static test_return_t connection_pool2_test(memcached_st *memc) // Release them.. for (size_t x= 0; x < POOL_SIZE; ++x) { - test_compare(MEMCACHED_SUCCESS, memcached_pool_push(pool, mmc[x])); + test_compare(MEMCACHED_SUCCESS, memcached_pool_release(pool, mmc[x])); } @@ -4344,22 +4278,137 @@ static test_return_t connection_pool2_test(memcached_st *memc) * of the connections in the pool. It should however be enabled * when I push the item into the pool */ - mmc[0]= memcached_pool_pop(pool, false, &rc); + mmc[0]= memcached_pool_fetch(pool, NULL, NULL); test_true(mmc[0]); test_compare(MEMCACHED_SUCCESS, memcached_pool_behavior_set(pool, MEMCACHED_BEHAVIOR_IO_MSG_WATERMARK, 9999)); - mmc[1]= memcached_pool_pop(pool, false, &rc); - test_true(mmc[1]); + { + memcached_return_t rc; + mmc[1]= memcached_pool_fetch(pool, NULL, &rc); + test_true(mmc[1]); + test_compare(MEMCACHED_SUCCESS, rc); + } test_compare(UINT64_C(9999), memcached_behavior_get(mmc[1], MEMCACHED_BEHAVIOR_IO_MSG_WATERMARK)); - test_compare(MEMCACHED_SUCCESS, memcached_pool_push(pool, mmc[1])); - test_compare(MEMCACHED_SUCCESS, memcached_pool_push(pool, mmc[0])); + test_compare(MEMCACHED_SUCCESS, memcached_pool_release(pool, mmc[1])); + test_compare(MEMCACHED_SUCCESS, memcached_pool_release(pool, mmc[0])); + + { + memcached_return_t rc; + mmc[0]= memcached_pool_fetch(pool, NULL, &rc); + test_true(mmc[0]); + test_compare(MEMCACHED_SUCCESS, rc); + } - mmc[0]= memcached_pool_pop(pool, false, &rc); test_compare(UINT64_C(9999), memcached_behavior_get(mmc[0], MEMCACHED_BEHAVIOR_IO_MSG_WATERMARK)); - test_compare(MEMCACHED_SUCCESS, memcached_pool_push(pool, mmc[0])); + test_compare(MEMCACHED_SUCCESS, memcached_pool_release(pool, mmc[0])); + + test_true(memcached_pool_destroy(pool) == memc); + + return TEST_SUCCESS; +} + +struct test_pool_context_st { + volatile memcached_return_t rc; + memcached_pool_st* pool; + memcached_st* mmc; + sem_t _lock; + + test_pool_context_st(memcached_pool_st *pool_arg, memcached_st *memc_arg): + rc(MEMCACHED_FAILURE), + pool(pool_arg), + mmc(memc_arg) + { + sem_init(&_lock, 0, 0); + } + + void wait() + { + sem_wait(&_lock); + } + + void release() + { + sem_post(&_lock); + } + + ~test_pool_context_st() + { + sem_destroy(&_lock); + } +}; + +static void* connection_release(void *arg) +{ + test_pool_context_st *resource= static_cast(arg); + assert(resource); + if (resource == NULL) + { + abort(); + } + + // Release all of the memc we are holding + resource->rc= memcached_pool_release(resource->pool, resource->mmc); + resource->release(); + + pthread_exit(arg); +} + +static test_return_t connection_pool3_test(memcached_st *memc) +{ + memcached_pool_st* pool= memcached_pool_create(memc, 1, 1); + test_true(pool); + + memcached_st *pool_memc; + { + memcached_return_t rc; + pool_memc= memcached_pool_fetch(pool, NULL, &rc); + test_compare(MEMCACHED_SUCCESS, rc); + test_true(pool_memc); + } + + /* + @note This comment was written to describe what was believed to be the original authors intent. + + This portion of the test creates a thread that will wait until told to free a memcached_st + that will be grabbed by the main thread. + + It is believed that this tests whether or not we are handling ownership correctly. + */ + pthread_t tid; + test_pool_context_st item(pool, pool_memc); + + test_zero(pthread_create(&tid, NULL, connection_release, &item)); + item.wait(); + + memcached_return_t rc; + memcached_st *pop_memc; + int counter= 5; + do + { + struct timespec relative_time= { 0, 0 }; + pop_memc= memcached_pool_fetch(pool, &relative_time, &rc); + + if (memcached_failed(rc)) + { + test_null(pop_memc); + } + } while (rc == MEMCACHED_TIMEOUT and --counter); + + if (memcached_failed(rc)) // Cleanup thread since we will exit once we test. + { + pthread_join(tid, NULL); + test_compare(MEMCACHED_SUCCESS, rc); + } + + { + int pthread_ret= pthread_join(tid, NULL); + test_true(pthread_ret == 0 or pthread_ret == ESRCH); + } + test_compare(MEMCACHED_SUCCESS, rc); + test_true(pool_memc == pop_memc); test_true(memcached_pool_destroy(pool) == memc); @@ -5910,6 +5959,7 @@ test_st tests[] ={ {"analyzer", true, (test_callback_fn*)analyzer_test}, {"memcached_pool_st", true, (test_callback_fn*)connection_pool_test }, {"memcached_pool_st #2", true, (test_callback_fn*)connection_pool2_test }, + {"memcached_pool_st #3", true, (test_callback_fn*)connection_pool3_test }, {"memcached_pool_test", true, (test_callback_fn*)memcached_pool_test }, {"test_get_last_disconnect", true, (test_callback_fn*)test_get_last_disconnect}, {"verbosity", true, (test_callback_fn*)test_verbosity}, -- 2.30.2