Fix apc_cache_default_expunge() to always remove expired entries

apc_cache.c

@@ -735,8 +735,7 @@ PHP_APCU_API void apc_cache_clear(apc_cache_t* cache)
 PHP_APCU_API void apc_cache_default_expunge(apc_cache_t* cache, size_t size)
 {
 	time_t t;
-	size_t suitable = 0L;
-	size_t available = 0L;
+	size_t i;
 
 	if (!cache) {
 		return;
@@ -751,43 +750,35 @@ PHP_APCU_API void apc_cache_default_expunge(apc_cache_t* cache, size_t size)
 		return;
 	}
 
-	/* make suitable selection */
-	suitable = (cache->smart > 0L) ? (size_t) (cache->smart * size) : (size_t) (cache->sma->size/2);
+	/* smart adjusts the required memory-size, to increase the probability of a full cache wipe */
+	size = (cache->smart > 0L) ? (size_t) (cache->smart * size) : size;
 
 	/* gc */
 	apc_cache_wlocked_gc(cache);
 
-	/* get available */
-	available = apc_sma_get_avail_mem(cache->sma);
-
-	/* check that expunge is necessary */
-	if (available < suitable) {
-		size_t i;
-
-		/* look for junk */
-		for (i = 0; i < cache->nslots; i++) {
-			apc_cache_entry_t **entry = &cache->slots[i];
-			while (*entry) {
-				if (apc_cache_entry_expired(cache, *entry, t)) {
-					apc_cache_wlocked_remove_entry(cache, entry);
-					continue;
-				}
-
-				/* grab next entry */
-				entry = &(*entry)->next;
+	/* remove expired entries */
+	for (i = 0; i < cache->nslots; i++) {
+		apc_cache_entry_t **entry = &cache->slots[i];
+		while (*entry) {
+			if (apc_cache_entry_expired(cache, *entry, t)) {
+				apc_cache_wlocked_remove_entry(cache, entry);
+				continue;
 			}
-		}
 
-		/* if the cache now has space, then reset last key */
-		if (apc_sma_get_avail_size(cache->sma, size)) {
-			/* wipe lastkey */
-			memset(&cache->header->lastkey, 0, sizeof(apc_cache_slam_key_t));
-		} else {
-			/* with not enough space left in cache, we are forced to expunge */
-			apc_cache_wlocked_real_expunge(cache);
+			/* grab next entry */
+			entry = &(*entry)->next;
 		}
 	}
 
+	/* if the cache now has space, then reset last key */
+	if (apc_sma_get_avail_size(cache->sma, size)) {
+		/* wipe lastkey */
+		memset(&cache->header->lastkey, 0, sizeof(apc_cache_slam_key_t));
+	} else {
+		/* with not enough space left in cache, we are forced to expunge */
+		apc_cache_wlocked_real_expunge(cache);
+	}
+
 	apc_cache_wunlock(cache);
 }
 /* }}} */

apc_cache.h

@@ -251,20 +251,12 @@ PHP_APCU_API void apc_cache_serializer(apc_cache_t* cache, const char* name);
 
 /* {{{ apc_cache_default_expunge
 * Where smart is not set:
-*  Where no ttl is set on cache:
-*   1) Perform cleanup of stale entries
-*   2) Expunge if available memory is less than sma->size/2
-*  Where ttl is set on cache:
-*   1) Perform cleanup of stale entries
-*   2) If available memory if less than the size requested, run full expunge
+*  1) Perform cleanup of stale entries
+*  2) If available memory is less than the size requested, run full expunge
 *
 * Where smart is set:
-*  Where no ttl is set on cache:
-*   1) Perform cleanup of stale entries
-*   2) Expunge is available memory is less than size * smart
-*  Where ttl is set on cache:
-*   1) Perform cleanup of stale entries
-*   2) If available memory if less than the size requested, run full expunge
+*  1) Perform cleanup of stale entries
+*  2) If available memory is less than the size requested * smart, run full expunge
 *
 * The TTL of an entry takes precedence over the TTL of a cache
 */

package.xml

@@ -73,6 +73,8 @@
     <file name="apc_023.phpt" role="test" />
     <file name="apc_024.phpt" role="test" />
     <file name="apc_025.phpt" role="test" />
+    <file name="apc_026.phpt" role="test" />
+    <file name="apc_027.phpt" role="test" />
     <file name="apc_099.phpt" role="test" />
     <file name="apc54_014.phpt" role="test" />
     <file name="apc54_018.phpt" role="test" />

tests/apc_026.phpt

@@ -0,0 +1,58 @@
+--TEST--
+Test TTL expiration with fragmented memory
+--SKIPIF--
+<?php
+require_once(__DIR__ . '/skipif.inc');
+if (!function_exists('apcu_inc_request_time')) die('skip APC debug build required');
+?>
+--INI--
+apc.enabled=1
+apc.enable_cli=1
+apc.use_request_time=1
+apc.ttl=1
+apc.shm_size=1M
+--FILE--
+<?php
+
+apcu_store("long_ttl", 123456789, 100);
+apcu_store("dummy", str_repeat('x', 11000));
+apcu_inc_request_time(2);
+
+// Fill the cache
+$i = 1;
+while (apcu_exists("dummy")) {
+    apcu_store("key_small_" . $i, $i, 3);
+    apcu_store("key_large_" . $i, str_repeat('x', 10000), 1);
+
+    $i++;
+}
+
+// Expire all large entries
+apcu_inc_request_time(2);
+
+// This insertion should trigger an expunge which expires all large entries
+var_dump(apcu_store("next_large_entry", str_repeat('x', 10000), 1));
+var_dump(apcu_fetch("next_large_entry") === str_repeat('x', 10000));
+
+// Check fragmented state (small entries are present, large entries are not present)
+var_dump(apcu_fetch("key_small_1"));
+var_dump(apcu_fetch("key_large_1"));
+
+// Expire all small entries
+apcu_inc_request_time(2);
+
+// This entry should trigger an expunge which expires all small entries.
+// This solves fragmentation which allows this entry to be stored without a full cache wipe.
+var_dump(apcu_store("very_large_entry", str_repeat('x', 20000), 1));
+var_dump(apcu_fetch("very_large_entry") === str_repeat('x', 20000));
+var_dump(apcu_fetch("long_ttl"));
+
+?>
+--EXPECT--
+bool(true)
+bool(true)
+int(1)
+bool(false)
+bool(true)
+bool(true)
+int(123456789)

tests/apc_027.phpt

@@ -0,0 +1,33 @@
+--TEST--
+Test expiration of entries when > 50% of memory is available
+--SKIPIF--
+<?php
+require_once(__DIR__ . '/skipif.inc');
+if (!function_exists('apcu_inc_request_time')) die('skip APC debug build required');
+?>
+--INI--
+apc.enabled=1
+apc.enable_cli=1
+apc.use_request_time=1
+apc.ttl=1
+apc.shm_size=1M
+--FILE--
+<?php
+
+
+apcu_store("key_01", 123, 100);
+apcu_store("key_02", str_repeat('x', 400000), 1);
+
+apcu_inc_request_time(2);
+
+var_dump(apcu_store("key_03", str_repeat('x', 900000), 1));
+var_dump(apcu_fetch("key_01"));
+var_dump(apcu_fetch("key_02"));
+var_dump(apcu_fetch("key_03") === str_repeat('x', 900000));
+
+?>
+--EXPECT--
+bool(true)
+int(123)
+bool(false)
+bool(true)