Skip to content

Commit 2192cea

Browse files
committed
Fixed #529 // Memory leak caused by item tags
1 parent d38ac62 commit 2192cea

File tree

5 files changed

+87
-2
lines changed

5 files changed

+87
-2
lines changed

src/phpFastCache/Cache/DriverBaseTrait.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -268,7 +268,7 @@ public function driverWriteTags(ExtendedCacheItemInterface $item)
268268
* then remove it from tagsItems index
269269
*/
270270
if (count($data)) {
271-
$tagsItem->expiresAt(max($data));
271+
$tagsItem->expiresAt((new \DateTime())->setTimestamp(max($data)));
272272
$this->driverWrite($tagsItem);
273273
$tagsItem->setHit(true);
274274
} else {

src/phpFastCache/Cache/ItemBaseTrait.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -131,7 +131,7 @@ public function expiresAt($expiration)
131131
if ($expiration instanceof \DateTimeInterface) {
132132
$this->expirationDate = $expiration;
133133
} else {
134-
throw new \InvalidArgumentException('$expiration must be an object implementing the DateTimeInterface');
134+
throw new phpFastCacheInvalidArgumentException('$expiration must be an object implementing the DateTimeInterface got: ' . gettype($expiration));
135135
}
136136

137137
return $this;

src/phpFastCache/Core/ExtendedCacheItemPoolTrait.php

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -350,6 +350,13 @@ protected function deregisterItem($item)
350350
gc_collect_cycles();
351351
}
352352
}
353+
/**
354+
* @param ExtendedCacheItemInterface $item
355+
*/
356+
protected function cleanItemTags(ExtendedCacheItemInterface $item)
357+
{
358+
$this->driverWriteTags($item->removeTags($item->getTags()));
359+
}
353360

354361
/**
355362
* Returns true if the item exists, is attached and the Spl Hash matches

src/phpFastCache/Core/StandardPsr6StructureTrait.php

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -164,6 +164,13 @@ public function deleteItem($key)
164164
*/
165165
$this->deregisterItem($key);
166166

167+
/**
168+
* Perform a tag cleanup to avoid memory leaks
169+
*/
170+
if (strpos($key, self::DRIVER_TAGS_KEY_PREFIX) !== 0) {
171+
$this->cleanItemTags($item);
172+
}
173+
167174
return true;
168175
}
169176

tests/issues/Github-529.test.php

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,71 @@
1+
<?php
2+
3+
/**
4+
* @author Khoa Bui (khoaofgod) <[email protected]> http://www.phpfastcache.com
5+
* @author Georges.L (Geolim4) <[email protected]>
6+
*/
7+
8+
use phpFastCache\CacheManager;
9+
use phpFastCache\Helper\TestHelper;
10+
11+
chdir(__DIR__);
12+
require_once __DIR__ . '/../../vendor/autoload.php';
13+
14+
/**
15+
* @param $obj
16+
* @param $prop
17+
* @return mixed
18+
* @throws \ReflectionException
19+
*/
20+
function accessInaccessibleMember($obj, $prop) {
21+
$reflection = new \ReflectionClass($obj);
22+
$property = $reflection->getProperty($prop);
23+
$property->setAccessible(true);
24+
return $property->getValue($obj);
25+
}
26+
27+
// Hide php Redis extension notice by using a little @
28+
$cacheInstance = CacheManager::getInstance('Files');
29+
$string = uniqid('pfc', true);
30+
31+
/**
32+
* Populate the cache with some data
33+
*/
34+
list($item, $item2) = array_values($cacheInstance->getItems(['item1', 'item2']));
35+
36+
$item->set($string)
37+
->addTags(['tag-all', 'tag1'])
38+
->expiresAfter(3600);
39+
40+
$item2->set($string)
41+
->addTags(['tag-all', 'tag2'])
42+
->expiresAfter(3600);
43+
44+
$cacheInstance->save($item);
45+
$cacheInstance->save($item2);
46+
$cacheInstance->detachAllItems();
47+
unset($item, $item2);
48+
49+
/**
50+
* Destroy the populated items
51+
*/
52+
$cacheInstance->deleteItemsByTag('tag-all');
53+
54+
/**
55+
* First test memory, as we will write the item inside in the second test
56+
*/
57+
$itemInstances = accessInaccessibleMember($cacheInstance, 'itemInstances');
58+
if (isset($itemInstances[$cacheInstance::DRIVER_TAGS_KEY_PREFIX . 'tag-all'])) {
59+
echo '[FAIL] The internal cache item tag is still stored in memory' . PHP_EOL;
60+
} else {
61+
echo '[PASS] The internal cache item tag is no longer stored in memory' . PHP_EOL;
62+
}
63+
64+
/**
65+
* Then test disk to see if the item is still there
66+
*/
67+
if ($cacheInstance->getItem($cacheInstance::DRIVER_TAGS_KEY_PREFIX . 'tag-all')->isHit()) {
68+
echo '[FAIL] The internal cache item tag is still stored on disk' . PHP_EOL;
69+
} else {
70+
echo '[PASS] The internal cache item tag is no longer stored on disk' . PHP_EOL;
71+
}

0 commit comments

Comments
 (0)