Ticket #4 (closed defect: fixed)

Opened 4 years ago

Last modified 4 years ago

memory leak in eaccelerator_get()

Reported by: mastabog@… Owned by: bart
Priority: major Milestone: 0.9.5
Component: eAccelerator Version: 0.9.5
Keywords: memory leak, shared memory, eaccelerator_get() Cc:

Description

Hello,

I believe I've hit a memory leak in eaccelerator_get() while trying to benchmark some memory caching classes i've built based on different packages/tools (memcache, native shm, eaccelerator, etc).

I'm using Debian sarge with apache 1.3, php 5.1.2 from dotdeb.org and the latest eaccelerator snapshot (eaccelerator-svn200603061029) compiled with --with-eaccelerator-shared-memory to enable key caching through eaccelerator_put() and eaccelerator_get().

The code below should not suffer any memory leaks but it does:

<?php

function ea ($str)
{
	eaccelerator_put("testkey", $str);

	for ($i = 0; $i < 1e5; $i++)
	{
		$r = eaccelerator_get("testkey");
	}

	return $r;
}

$str = str_repeat("1234567890", 1024); // 10 KB
$r = ea($str);
echo strlen($r);

?>

Yields:

Fatal error: Allowed memory size of 8388608 bytes exhausted (tried to allocate 10241 bytes) in /var/www/test/ea2.php on line 9

The native php SysV shared memory module works just fine:

<?php

function shm ($str)
{
	$shm_key = ftok(__FILE__, chr(1));
	$shm_id  = shm_attach($shm_key, 256 * 1024, 0666); // a 256 KB block
	shm_put_var($shm_id, 1, $str);

	for ($i = 0; $i < 1e5; $i++)
	{
		$r = shm_get_var($shm_id, 1);
	}

	return $r;
}

$str = str_repeat("1234567890", 1024); // 10 KB
$r = shm($str);
echo strlen($r);

?>

Yields:

10240

as expected.

I saw eaccelerator_get() defined in cache.c is used in the session functions too when EA is set as the session handler but also in content caching and this memory leak looks quite nasty!

Anyway, eaccelator is a superb extension. Beats all other php cachers/optimizers I've tried.

Thank you and keep up the good work!

Change History

comment:1 Changed 4 years ago by bart

  • Owner changed from somebody to bart
  • Status changed from new to assigned

I'm not shure if this is a memoryleak. Everytime a key is returned from memory a copy is returned and not a reference. This means you're copying the value everytime and also allocating memory for it.

comment:2 Changed 4 years ago by bart

  • Status changed from assigned to closed
  • Resolution set to fixed

I found the problem. When a value was stored it's refcount was kept. So even when there wasn't a reference in the script anymore the old references from when it was stored were still kept. I modified the caching code to set the reference count to 1 when it returns the value which is the correct value. This is fixed in changeset [180]. I've also updated the snapshots at  http://snapshots.eaccelerator.net

comment:3 Changed 4 years ago by mastabog@…

I took a look at the function code in cache.c and came here to suggest the fix but you beat me to it :).

Tested the new svn200603071800 snapshot and it works fine. Nicely done, thanks!

Note: See TracTickets for help on using tickets.