# 16-bit reference count in PHP 4's zval - Thread: https://externals.io/message/21441 - Original archive: https://news-web.php.net/php.internals/21441 - Started by Tim Starling on 2006-01-07T04:47:27+00:00 - Messages: 8 - Reply by email to internals@lists.php.net - More for agents: search with https://externals.io/search.md?q={query} · JSON API at https://externals.io/api · documentation at https://externals.io/llms.txt --- ## #21441 — Tim Starling — 2006-01-07T04:47:27+00:00 URL: https://externals.io/message/21441#21441 I've had some trouble with large arrays in my PHP programs causing corruption of the heap. I tracked the problem back to PHP 4's 16-bit reference count. If you have more than 64K references to a given zval, the counter will overflow, then when the references are freed, the object will be double-freed, causing a segfault. Dangling references are also possible, allowing reading and writing of subsequently allocated blocks. No doubt the PHP dev team are aware of this already, since it was fixed in PHP 5, by using a 32-bit type instead. My question is: is there any intention to backport this simple but important bugfix to PHP 4? Many PHP users are still using PHP 4, and it's not a very well advertised fact that it does not properly support arrays with more than 64K entries. -- Tim Starling --- ## #21442 — Marcus Boerger — 2006-01-07T11:24:39+00:00 URL: https://externals.io/message/21441#21442 · In reply to: #21441 Hello Tim, that's a major API break so we would need to make that 4.5. Which is kind of out of question atm. Just change to 5. marcus Saturday, January 7, 2006, 5:47:27 AM, you wrote: > I've had some trouble with large arrays in my PHP programs causing > corruption of the heap. I tracked the problem back to PHP 4's 16-bit > reference count. If you have more than 64K references to a given zval, the > counter will overflow, then when the references are freed, the object will > be double-freed, causing a segfault. Dangling references are also possible, > allowing reading and writing of subsequently allocated blocks. > No doubt the PHP dev team are aware of this already, since it was fixed in > PHP 5, by using a 32-bit type instead. My question is: is there any > intention to backport this simple but important bugfix to PHP 4? Many PHP > users are still using PHP 4, and it's not a very well advertised fact that > it does not properly support arrays with more than 64K entries. > -- Tim Starling Best regards, Marcus --- ## #21447 — Zeev Suraski — 2006-01-08T15:12:35+00:00 URL: https://externals.io/message/21441#21447 · In reply to: #21441 At 06:47 07/01/2006, Tim Starling wrote: >I've had some trouble with large arrays in my PHP programs causing >corruption of the heap. I tracked the problem back to PHP 4's 16-bit >reference count. If you have more than 64K references to a given zval, the >counter will overflow, then when the references are freed, the object will >be double-freed, causing a segfault. Dangling references are also possible, >allowing reading and writing of subsequently allocated blocks. > >No doubt the PHP dev team are aware of this already, since it was fixed in >PHP 5, by using a 32-bit type instead. My question is: is there any >intention to backport this simple but important bugfix to PHP 4? Many PHP >users are still using PHP 4, and it's not a very well advertised fact that >it does not properly support arrays with more than 64K entries. Tim, Your analysis was correct until the last sentence - PHP surely does support arrays with more than 64K entries. It just doesn't support the same entry being linked from more than 64K locations (which is much, much more rare occurrence). Zeev --- ## #21448 — Tim Starling — 2006-01-08T22:00:41+00:00 URL: https://externals.io/message/21441#21448 · In reply to: #21447 Zeev Suraski wrote: > Tim, > > Your analysis was correct until the last sentence - PHP surely does > support arrays with more than 64K entries. It just doesn't support the > same entry being linked from more than 64K locations (which is much, > much more rare occurrence). Well yes, I was just stirring. It's more common than you might think though. My own case was somewhat similar to this: name = $name; } } $cache = array(); for ( $i=0; $i<100000; $i++) { $cache[] = new CacheEntry( "Cache entry $i" ); } ?> ...which will segfault on exit due to the excess references to the empty array object created for the initialisation of the unused member variable $stuff. Generally speaking, arrays of objects with initialised but unused member variables will cause problems. Our code (MediaWiki) has plenty of such arrays, the reason we don't see this more often is because the arrays usually contain less than 64K entries. The size of the arrays depend on user input. -- Tim Starling --- ## #21449 — Zeev Suraski — 2006-01-09T07:52:13+00:00 URL: https://externals.io/message/21441#21449 · In reply to: #21448 At 00:00 09/01/2006, Tim Starling wrote: >Zeev Suraski wrote: > > […] > >Well yes, I was just stirring. It's more common than you might think though. I'm not saying that it's four-leafed clover, just that it's significantly more rare than plain arrays that contain 64K (different) entries... We did decide to increase the refcount size in PHP 5, but as Marcus said, releasing a binary-compat-breaking version of 4 at this stage is pretty much out of the question. Zeev --- ## #21454 — Alan Pinstein — 2006-01-09T18:57:32+00:00 URL: https://externals.io/message/21441#21454 · In reply to: #21448 It seems that the problem you're encountering could be fixed trivially by moving the initialization of $stuff into the constructor. However, it's difficult to tell from the provided code whether $stuff is intended as a static member... but if it's not, seems like a very simple fix... I don't personally know about PHP4 internals, but it would seem that the reason it breaks as-is is that ALL CacheEntry objects initially report to the same (statically created) empty array. So, moving it into the constructor would at least create n empty arrays rather than segfaulting. Right? Then you wouldn't even need to change your runtime environment (ie PHP version) to solve the problem. Alan > Zeev Suraski wrote: > > […] > > Well yes, I was just stirring. It's more common than you might > think though. > My own case was somewhat similar to this: > > class CacheEntry { > var $name; > var $stuff = array(); > > function CacheEntry( $name ) { > $this->name = $name; > } > } > > $cache = array(); > for ( $i=0; $i<100000; $i++) { > $cache[] = new CacheEntry( "Cache entry $i" ); > } > ?> > > ...which will segfault on exit due to the excess references to the > empty > array object created for the initialisation of the unused member > variable > $stuff. Generally speaking, arrays of objects with initialised but > unused > member variables will cause problems. Our code (MediaWiki) has > plenty of > such arrays, the reason we don't see this more often is because the > arrays > usually contain less than 64K entries. The size of the arrays > depend on user > input. > > -- Tim Starling --- ## #21456 — Tim Starling — 2006-01-10T01:33:29+00:00 URL: https://externals.io/message/21441#21456 · In reply to: #21454 Alan Pinstein wrote: > It seems that the problem you're encountering could be fixed trivially > by moving the initialization of $stuff into the constructor. > > However, it's difficult to tell from the provided code whether $stuff > is intended as a static member... but if it's not, seems like a very > simple fix... > > I don't personally know about PHP4 internals, but it would seem that > the reason it breaks as-is is that ALL CacheEntry objects initially > report to the same (statically created) empty array. So, moving it into > the constructor would at least create n empty arrays rather than > segfaulting. > > Right? Then you wouldn't even need to change your runtime environment > (ie PHP version) to solve the problem. The easy way to solve it was to limit the array to less than 64K entries, which I did before I made my original post. The problem only showed up because of a bug causing the cache to grow in an unconstrained way. Thanks for the suggestion, but the reason I started this thread was because I wanted to make sure that this doesn't happen again, regardless of bad programming. Upgrading to PHP 5 is certainly a reasonable solution, it's been on our wishlist for a while. We will probably drop support for PHP 4 in MediaWiki 1.6 or 1.7, so that we can take advantage of the new features. Upgrading won't be a trivial task, we're apparently one of the larger PHP installations in the world, with about 100 servers running PHP, and there's a number of custom extensions to check as well. Still, it has to be done some time. -- Tim Starling --- ## #21457 — Jani Taskinen — 2006-01-10T07:40:39+00:00 URL: https://externals.io/message/21441#21457 · In reply to: #21456 > Upgrading to PHP 5 is certainly a reasonable solution, it's been on our > wishlist for a while. We will probably drop support for PHP 4 in MediaWiki > 1.6 or 1.7, so that we can take advantage of the new features. Upgrading We've used MediaWiki at work with PHP 5.1.x (from CVS) for a long time without any problems. I'd suggest you just start another branch of it e.g. 2.0 which is for PHP 5.1 only. Then just don't upgrade 1.x anymore. :) People will just have to upgrade. They just don't know it's for their own good yet.. --Jani