I am starting a series of threads on best practices, coding guidelines, architecture, and generally getting the most out of Sugar Suite.
This first thread is about reference usage in PHP and stability issues that we addressed in the 3.5.1 release. Coming soon will be guidelines on a list of changes that should be made to all PHP code and custom modules developed for Sugar Suite.
Jacob Taylor
CTO SugarCRM.com
Engineering/PHP Reference Usage Overview
PHP Stability issues. In the past, there have been several incidents reported where there would be problems using Sugar Suite. Some of the symptoms reported include:
- Getting Apache Segmentation Faults (or error dialog if running on Windows)
- Clicking on a link in the UI and getting no response, a partial response, or a HTTP 404 error
- Clicking on a link in the UI and getting a PHP error saying it could not allocate a huge amount of memory (~1.6GB)
Similar experiences of unexplained behavior using the Sugar Plug-in for Outlook, causing Apache segmentation faults or an Apache error dialog popping up (if Apache is running on Windows). At this point, we have traced down several causes for these problems. There is a general issue with the use of references in PHP where you can experience memory corruption. There was also one specific section of code that triggered memory corruption problems.
With the release of Sugar Suite 3.5.1, every known bad usages of references have been fixed. If you have any reproducable use case on top of 3.5.1 please let us know.
The code below attempts to show good and bad uses of references. Basically all references must come from variables or a function that returns a variable by reference to be safe.
PHP Code:
<?php
// Assignment by reference is not safe
function bad_assignment()
{
$temp_ref =& function_that_returns_something_by_value();
}
// drop the ampersand from the function declaration. In php 5.0 it will
// create a reference to the object. In PHP 4.X it will copy the data.
function good_assignments()
{
$temp_ref =& new SugarBean(); // PHP 4.x only In PHP 5 this is
// depricated but should still be safe
$temp_ref = new SugarBean(); // PHP 4.x will make a copy, PHP 5
// will effectively create a reference
// (copy of the object handle)
$temp_ref = function_that_returns_something();
$temp_ref =& function_that_returns_something_by_reference();
$temp_ref =& $another_var;
}
// The following returns are bad, because they are not variables.
// Returning a reference from a function requires a variable
function &bad_return()
{
return 'test'.'return';
return 'test';
return function_that_returns_something_by_value();
return null; // this one is rather important.
// It is bad, but is not obvious.
}
// There are two ways to fix this
// or drop the "reference" from the return
function good_return()
{
; // insert same content as in bad_return
}
// or assign the value to a variable before returning
function &good_return2()
{
$temp = 'test'.'return';
return $temp;
$temp = 'test';
return $temp;
$temp = function_that_returns_something();
return $temp;
return function_that_returns_something_by_ref();
}
// passing a variable to a function by reference
function no_ref($arg)
{
//arg will be passed by value.
}
// Keep in mind at least in PHP 4.x passing a variable by reference
// makes a copy and then copies it back. If you are not modifying
// the variable or storing a reference to it in the function, you
// should not pass it by reference.
function declaration_ref(&$arg)
{
//arg will be passed by reference.
}
// good calls
no_ref($variable);
declaration_ref($variable); //normal pass by ref
// bad call "call time pass by reference". This is a deprecated method
// in PHP. If you need to pass a reference, declare it in the function.
no_ref(&$variable);
declaration_ref(&$variable);
// PHP 5 makes a change to the way the ‘=’ operator is used with
// objects. Since the equals operator in PHP 4 copies objects
// and the equals operator in PHP 5 creates references to objects,
// you might run into some issues.
$temp = new SugarBean();
for($i=0; $i<100; $i++)
{
$temp->id = $i;
$array[$i] = $temp; // This assigns a reference to $temp
// variable. You end up with a list that is all
// the same as the last element.
}
// Fix.
$temp = new SugarBean();
for($i=0; $i<100; $i++)
{
if (substr(phpversion(), 0, 1) == "5") {
$array[$i] = clone $temp;
}
else
{
$array[$i] = $temp;
}
}
?>
The other code that caused stability issues was in soap/SoapHelperFunctions (may not be in all versions):
PHP Code:
function get_field_list(&$value){
...
foreach($options_dom as $key=>$oneOption)
In some instances, the $options_dom variable was a string. This API was not intended to have just a string in that location, it was supposed to be an array. Normally, this would only be a PHP error message. For some reason, in this case, it was consistently causing a segmentation fault. To fix this code, we added:
PHP Code:
if(!is_array($options_dom)) $options_dom = array();
Just before the foreach above in 3.5.1.
Bookmarks