Changed behaviour of Memcache::close() to actually remove all servers from pool instead of just closing the connections

memcache.c

@@ -1801,13 +1801,7 @@ PHP_FUNCTION(memcache_close)
 		RETURN_FALSE;
 	}
 
-	for (i=0; i<pool->num_servers; i++) {
-		if (!pool->servers[i]->persistent) {
-			mmc_server_disconnect(pool->servers[i], &(pool->servers[i]->tcp) TSRMLS_CC);
-			mmc_server_disconnect(pool->servers[i], &(pool->servers[i]->udp) TSRMLS_CC);
-		}
-	}
-
+	mmc_pool_close(pool TSRMLS_CC);
 	RETURN_TRUE;
 }
 /* }}} */

memcache_pool.c

@@ -750,20 +750,10 @@ void mmc_server_free(mmc_t *mmc TSRMLS_DC) /* {{{ */
 }
 /* }}} */
 
-mmc_pool_t *mmc_pool_new(TSRMLS_D) /* {{{ */
+static void mmc_pool_init_hash(mmc_pool_t *pool TSRMLS_DC) /* {{{ */
 {
 	mmc_hash_function hash;
-	mmc_pool_t *pool = emalloc(sizeof(mmc_pool_t));
-	memset(pool, 0, sizeof(*pool));
 
-	switch (MEMCACHE_G(protocol)) {
-		case MMC_BINARY_PROTOCOL:
-			pool->protocol = &mmc_binary_protocol;
-			break;
-		default:
-			pool->protocol = &mmc_ascii_protocol;
-	}
-	
 	switch (MEMCACHE_G(hash_strategy)) {
 		case MMC_CONSISTENT_HASH:
 			pool->hash = &mmc_consistent_hash;
@@ -781,8 +771,25 @@ mmc_pool_t *mmc_pool_new(TSRMLS_D) /* {{{ */
 	}
 	
 	pool->hash_state = pool->hash->create_state(hash);
-	pool->min_compress_savings = MMC_DEFAULT_SAVINGS;
+}
+/* }}} */
+
+mmc_pool_t *mmc_pool_new(TSRMLS_D) /* {{{ */
+{
+	mmc_pool_t *pool = emalloc(sizeof(mmc_pool_t));
+	memset(pool, 0, sizeof(*pool));
+
+	switch (MEMCACHE_G(protocol)) {
+		case MMC_BINARY_PROTOCOL:
+			pool->protocol = &mmc_binary_protocol;
+			break;
+		default:
+			pool->protocol = &mmc_ascii_protocol;
+	}
 	
+	mmc_pool_init_hash(pool TSRMLS_CC);
+	pool->min_compress_savings = MMC_DEFAULT_SAVINGS;
+
 	pool->sending = &(pool->_sending1);
 	pool->reading = &(pool->_reading1);
 
@@ -838,6 +845,31 @@ void mmc_pool_add(mmc_pool_t *pool, mmc_t *mmc, unsigned int weight) /*
 }
 /* }}} */
 
+void mmc_pool_close(mmc_pool_t *pool TSRMLS_DC) /* 
+	disconnects and removes all servers in the pool {{{ */
+{
+	if (pool->num_servers) {
+		int i;
+		
+		for (i=0; i<pool->num_servers; i++) {
+			if (pool->servers[i]->persistent) {
+				mmc_server_sleep(pool->servers[i] TSRMLS_CC);
+			} else {
+				mmc_server_free(pool->servers[i] TSRMLS_CC);
+			}
+		}
+
+		efree(pool->servers);
+		pool->servers = NULL;
+		pool->num_servers = 0;
+		
+		/* reallocate the hash strategy state */
+		pool->hash->free_state(pool->hash_state);
+		mmc_pool_init_hash(pool TSRMLS_CC);
+	}
+}
+/* }}} */
+
 int mmc_pool_open(mmc_pool_t *pool, mmc_t *mmc, mmc_stream_t *io, int udp TSRMLS_DC) /* 
 	connects if the stream is not already connected {{{ */
 {

memcache_pool.h

@@ -299,6 +299,7 @@ int mmc_request_failure(mmc_t *, mmc_stream_t *, const char *, unsigned int, int
 mmc_pool_t *mmc_pool_new(TSRMLS_D);
 void mmc_pool_free(mmc_pool_t * TSRMLS_DC);
 void mmc_pool_add(mmc_pool_t *, mmc_t *, unsigned int);
+void mmc_pool_close(mmc_pool_t * TSRMLS_DC);
 int mmc_pool_open(mmc_pool_t *, mmc_t *, mmc_stream_t *, int TSRMLS_DC);
 void mmc_pool_select(mmc_pool_t *, long TSRMLS_DC);
 void mmc_pool_run(mmc_pool_t * TSRMLS_DC);

package2.xml

@@ -37,6 +37,7 @@ http://pear.php.net/dtd/package-2.0.xsd">
   <license uri="http://www.php.net/license">PHP License</license>
   <notes>
     - Fixed PECL bug #12866 (Ignore memcache.default_port in memcache_connect)
+    - Changed behaviour of close() to actually remove all servers from pool (PECL bug #12555)
     - Added configure option for PEAR installer to disable session handler support
   </notes>
   <contents>

tests/024.phpt

@@ -15,16 +15,22 @@ var_dump($result1);
 
 $memcache = new Memcache();
 $result2 = $memcache->connect($host, $port);
-$result3 = $memcache->set('non_existing_test_key', 'test', false, 1);
+$result3 = $memcache->set('test_key', 'test', false, 1);
 $result4 = $memcache->close();
 
-// This additional get() will transparently reconnect
-$result5 = $memcache->get('non_existing_test_key');
+// This will fail since all servers have been removed
+$result5 = $memcache->get('test_key');
+
+// Reconnect server
+$result6 = $memcache->connect($host, $port);
+$result7 = $memcache->get('test_key');
 
 var_dump($result2);
 var_dump($result3);
 var_dump($result4);
 var_dump($result5);
+var_dump($result6);
+var_dump($result7);
 
 ?>
 --EXPECT--
@@ -32,4 +38,6 @@ bool(true)
 bool(true)
 bool(true)
 bool(true)
+bool(false)
+bool(true)
 string(4) "test"

tests/024a.phpt

@@ -15,11 +15,11 @@ var_dump($result1);
 
 $memcache = new Memcache();
 $result2 = $memcache->connect($host, $port);
-$result3 = memcache_set($memcache, 'non_existing_test_key', 'test', false, 1);
+$result3 = memcache_set($memcache, 'test_key', 'test', false, 1);
 $result4 = memcache_close($memcache);
 
-// This additional get() will transparently reconnect
-$result5 = memcache_get($memcache, 'non_existing_test_key');
+// This will fail since all servers have been removed
+$result5 = memcache_get($memcache, 'test_key');
 
 var_dump($result2);
 var_dump($result3);
@@ -40,7 +40,7 @@ bool(true)
 bool(true)
 bool(true)
 bool(true)
-string(4) "test"
+bool(false)
 bool(true)
 bool(true)
 

tests/052.phpt

@@ -0,0 +1,36 @@
+--TEST--
+memcache->connect() and memcache->close() in loop
+--SKIPIF--
+<?php include 'connect.inc'; ?>
+--FILE--
+<?php
+
+include 'connect.inc';
+
+$start = time();
+$count = 100;
+
+$memcache = new Memcache();
+$memcache->connect($host, $port, false);
+
+$key = 'test_key';
+$memcache->set($key, 'test');
+$memcache->close();
+
+for ($i=0; $i<$count; $i++) {
+	$memcache->connect($host, $port, false);
+	$result = $memcache->get($key);
+	
+	if (!$result) {
+		printf('Failed to fetch value for iteration %d', $i);
+	}
+	
+	$memcache->close();
+}
+
+$end = time();
+printf("%d seconds", $end - $start);
+
+?>
+--EXPECT--
+0 seconds

tests/100a.phpt

@@ -37,7 +37,7 @@ echo "Done\n";
 bool(true)
 bool(false)
 bool(false)
-bool(false)
+bool(true)
 
 Warning: memcache_flush() expects parameter 1 to be MemcachePool, object given in %s on line %d
 NULL