CRM Open Source Business & Social CRM Software

Results 1 to 6 of 6

Thread: PHP Reference Usage and Stability

  1. #1
    Jacob's Avatar
    Jacob is offline Senior Member
    Join Date
    Oct 2004
    Posts
    331

    Default PHP Reference Usage and Stability [CTO Series]

    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(), 01) == "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.
    Last edited by Jacob; 2005-10-23 at 11:49 PM. Reason: Updating use case for =& new Object to be safe.

  2. #2
    Eraserhd is offline Member
    Join Date
    Sep 2005
    Location
    Cleveland, Ohio
    Posts
    9

    Default Re: PHP Reference Usage and Stability [CTO Series]

    I work with the Horde project a bunch, and everything you've mentioned appears to be in agreement with their thinking, except that the Horde project uses:

    $foo = &new Object();

    (Apparently, PHP 4 will copy the object right after construction. If you have some sort of destructor-type of mechanism that you initialize in the constructor (like the PEAR stuff does), you'll find you are not destroying the same object that you are working with.)

    Does this disagree with something that you came up with?

  3. #3
    Jacob's Avatar
    Jacob is offline Senior Member
    Join Date
    Oct 2004
    Posts
    331

    Default Re: PHP Reference Usage and Stability [CTO Series]

    I am looking into the use case that you questioned.

    I will hopefully have an official response today.

    Keep in mind that some of the reference functionality worked without being safe. For now, I have been tending to be over cautious to keep stability as high as possible.

    Jacob

  4. #4
    LooseBruce is offline Member
    Join Date
    Oct 2005
    Posts
    7

    Default Re: PHP Reference Usage and Stability [CTO Series]

    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:
    Is this the kind of error that you're referring to Jacob? Happens today after a new install of 3.5.1a

    [Sat Oct 15 13:54:25 2005] [error] PHP Notice: Array to string conversion in /Library/WebServer/Documents/demval/sugar/include/database/PearDatabase.php on line 574
    [Sat Oct 15 14:08:23 2005] [error] PHP Notice: Array to string conversion in /Library/WebServer/Documents/demval/sugar/include/database/PearDatabase.php on line 574
    [Sat Oct 15 14:12:02 2005] [error] PHP Notice: Array to string conversion in /Library/WebServer/Documents/demval/sugar/include/database/PearDatabase.php on line 574

    Thanks,
    LB

  5. #5
    Jacob's Avatar
    Jacob is offline Senior Member
    Join Date
    Oct 2004
    Posts
    331

    Default Re: PHP Reference Usage and Stability [CTO Series]

    This is actually the opposite of the error that caused the problem.

    This is taking an array and storing it in a text field (array to string conversion). The other case was taking a string and treating it like an array. The value was always supposed to be an array but for a few instances it held a string instead.

    I have filed a bug on the array to string conversion notices.

    Can you tell how you reproduce the problem? That should help us fix it.

    For production use, we recommend running with notices turned off.

    Thanks,
    Jacob

  6. #6
    Jacob's Avatar
    Jacob is offline Senior Member
    Join Date
    Oct 2004
    Posts
    331

    Default Re: PHP Reference Usage and Stability [CTO Series]

    Quote Originally Posted by Eraserhd
    I work with the Horde project a bunch, and everything you've mentioned appears to be in agreement with their thinking, except that the Horde project uses:

    $foo = &new Object();

    (Apparently, PHP 4 will copy the object right after construction. If you have some sort of destructor-type of mechanism that you initialize in the constructor (like the PEAR stuff does), you'll find you are not destroying the same object that you are working with.)

    Does this disagree with something that you came up with?

    I have finally gotten a definative answer. You are correct.

    $foo = new Object();

    PHP 4.x will make a copy.
    PHP 5.x will copy the object handle (effectively making a reference).

    $foo =& new Object()

    PHP 4.x is safe and will not make a copy
    PHP 5.x is depricated. It should still work fine.

    Thanks,
    Jacob

Thread Information

Users Browsing this Thread

There are currently 1 users browsing this thread. (0 members and 1 guests)

Bookmarks

Posting Permissions

  • You may not post new threads
  • You may not post replies
  • You may not post attachments
  • You may not edit your posts
  •