-1

I have just added a new query to my webpage that uses a while loop to produce an array of the results. However when I try to run the page I get the error: "Fatal error: Allowed memory size of 134217728 bytes exhausted (tried to allocate 64 bytes) on line 305". I cannot work out why this has suddenly occurred, or what the memory leak might be. I do not wish to increase the size of the PHP memory_limit.

My question is different... I am asking for advice on what might be causing the memory leak, not what this fatal error is or means.

Query in question:

<?php

        $result1 = $con->query("SELECT SkillID FROM userskills WHERE UserID = '$User'") or die(mysqli_error($con));

        $current_skills = array();

        while (($skillrow = mysqli_fetch_array($result1, MYSQLI_NUM)) !== false){
            $current_skills[] = $skillrow;
            }
?>

Full Page:

<?php 

error_reporting(E_ALL); ini_set('display_errors', 1); 
require 'Assets/Connections/Connections.php';
session_start(); 
    if(isset($_SESSION["UserID"]))
    {
    }
    else
    {
        header('Location: LogIn.php');
        die();
    }

    $User = (int)$_SESSION["UserID"];

    $result = $con->query("SELECT * FROM user WHERE UserID ='$User'") or die(mysqli_error($con));

    $row = $result->fetch_array(MYSQLI_BOTH);

    $_SESSION["FirstName"] = $row['Fname'];
    $_SESSION["LastName"] = $row['Lname'];
    $_SESSION["Email"] = $row['Email'];
    $_SESSION["Role"] = $row['JobRole'];

    $skillresult = $con->query("SELECT userskills.SkillID, Description, Experience FROM User INNER JOIN userskills ON User.UserId = userskills.UserId JOIN Skills ON userskills.SkillID = Skills.SkillID WHERE user.UserID ='$User'") 
    or die(mysqli_error($con));

    $skills_array = array();

    while($r=mysqli_fetch_array($skillresult)){
    if (!isset($skills_array[$r['SkillID']])){
        $skills_array[$r['SkillID']] = array();
    }
    $skills_array[$r['SkillID']][] = $r['Description'];
}

    if(isset($_POST['Update']))
    {

        $UpdateFName = $_SESSION["FirstName"];
        if ($_POST['FirstName'] != '' ) { $UpdateFName = $_POST['FirstName'];}
        $UpdateLName = $_SESSION["LastName"];
        if ($_POST['LastName'] != '' ) { $UpdateLName = $_POST['LastName'];}
        $UpdateEmail = $_SESSION["Email"];
        if ($_POST['Email'] != '' ) { $UpdateEmail = $_POST['Email'];}
        $UpdateRole = $_SESSION["Role"];
        if ($_POST['JobRole'] != '' ) { $UpdateRole = $_POST['JobRole'];}
        $PasswordCheck = $_POST['Password'];
        if(password_verify($PasswordCheck, $row['Password']))
        {

            $sql = $con->query("UPDATE user SET 
                Fname = '{$UpdateFName}', 
                Lname = '{$UpdateLName}', 
                Email = '{$UpdateEmail}', 
                JobRole = '{$UpdateRole}'
            WHERE UserID = $User") or die(mysqli_error($con));

            if(!empty($_FILES['file']['name']))
            {
                $file = basename($_FILES['file']['name']);
                move_uploaded_file($_FILES['file']['tmp_name'], 'Assets/Images/'.$file);
            }

            if(isset($file))
            {
                $sql = $con->query("UPDATE user SET ProfileImage = '".$_FILES['file']['name']."' WHERE UserID = $User") or die(mysqli_error($con));
            }

            $default = 0;

            foreach($skills_array AS $skills_id=>$skills_name)
            {
                if (isset($_POST[$skills_name]))
                {
                    if (empty($_POST[$skills_name.'exp']))
                    {
                        $exp = $default;
                    }
                    else
                    {
                        $exp = $_POST[$skills_name.'exp'];
                    }

                    $sql = $con->query("SELECT count(UserID) as total FROM userskills WHERE UserID = $User AND SkillID = ".$skills_id) or die(mysqli_error($con));

                    if ($row = mysqli_fetch_assoc($sql))
                    {
                        $sql = $con->query("INSERT INTO userskills ( UserID, SkillID, Experience) VALUES  ($User, $skills_id, $exp)");
                        //If the checkbox is not checked it will check to see if skill is already a skill assigned to the user. If they are it will delete it. If not it will ignore.   
                    }
                    else
                    {
                        $sql = $con->query("UPDATE userskills SET Experience = $exp WHERE UserID = $User AND SkillID = ".$skills_id);
                    }
                } 
                else
                {
                    $sql = $con->query("DELETE FROM userskills WHERE UserID = $User AND SkillID = ".$skills_id);
                }
            }

            header('Location: Account.php');
            die();
        }
        else
        {
            echo 'Incorrect password please try again.';
        }
    }
?>
<!doctype html>
<html>
<head>
<link href="Assets/CSS/Master.css" rel="stylesheet" type="text/css" />
<link href="Assets/CSS/Menu.css" rel="stylesheet" type="text/css" />
<meta charset="utf-8">
<title>Update Account</title>
</head>

<body>
<div class="Container">
        <div class="Header"></div>
        <div class="Menu">
                <div id="Menu">
                        <nav>
                                <ul class="cssmenu">
                                        <li><a href="Home.php">Home</a></li>
                                        <li><a href="Account.php">Account</a></li>
                                        <li><a href="Projects.php">Projects</a></li>
                                        <li><a href="Users.php">Users</a></li>
                                        <li><a href="LogOut.php">LogOut</a></li>
                                </ul>
                        </nav>
                </div>
        </div>
        <div class="LeftBody">
                <form id="form1" name="form1" method="post" enctype="multipart/form-data">
                        <div class="FormElement">
                                <input name="FirstName" type="text" class="TField" id="FirstName" placeholder="First Name" value="<?php echo $_SESSION["FirstName"]; ?>">
                        </div>
                        <div class="FormElement">
                                <input name="LastName" type="text" class="TField" id="LastName" placeholder="Last Name" value="<?php echo $_SESSION["LastName"]; ?>">
                        </div>
                        <div class="FormElement">
                                <input name="Email" type="email" class="TField" id="Email" placeholder="Email Address" value="<?php echo $_SESSION["Email"]; ?>">
                        </div>
                        <div class="FormElement">
                                <input name="JobRole" type="text" class="TField" id="JobRole" placeholder="Job Role" value="<?php echo $_SESSION["Role"]; ?>">
                        </div>
                        <div class="FormElement">
                                <input name="Password" type="password" class="TField" id="Password" placeholder="Password" required="requried">
                        </div>
                        <div class="FormElement">
                                <input type="file" name="file">
                                <br>
                                <br>
                        </div>
                        <div class="FormElement">
                                <input name="Update" type="submit" class="button" id="Update" value="Submit Changes">
                        </div>
                </form>
        </div>
        <div class="RightBody">
                <form id="form2" name="form2" method="post" enctype="multipart/form-data">
                        <p><h3>Skills:</h3>
        <?php

        //advice given from stackoverflow. Suggests looping around the results of this to output 
        $result1 = $con->query("SELECT skills.SkillID, skills.Description, COUNT(userskills.SkillID) AS SkillUserHas, MAX(Experience) AS Experience
                                FROM 
                                (
                                     SELECT 1 AS SkillID, 'Java' AS Description
                                     UNION
                                     SELECT 7 AS SkillID, 'iOS' AS Description
                                     UNION
                                     SELECT 9 AS SkillID, 'PHP' AS Description
                                     UNION
                                     SELECT 3 AS SkillID, 'SQL' AS Description
                                     UNION
                                     SELECT 4 AS SkillID, 'Windows' AS Description
                                     UNION
                                     SELECT 5 AS SkillID, 'Linux' AS Description
                                     UNION
                                     SELECT 6 AS SkillID, 'Unix' AS Description
                                     UNION
                                     SELECT 8 AS SkillID, 'Requirements Elicitation' AS Description
                                ) skills
                                LEFT OUTER JOIN userskills
                                ON skills.SkillID = userskills.SkillID AND userskills.UserID = '$User'
                                GROUP BY skills.SkillID, skills.Description
                                ORDER BY FIELD(skills.SkillID, 1, 7, 9, 3, 4, 5, 6, 8)") 
                                or die(mysqli_error($con));



        while ($skillrow = $result1->fetch_assoc()) 
        {
        ?>
                        <div class="CheckboxText">
                        <?php
                            echo '<label>';
                            echo '<input type="checkbox" name="'.$skillrow['Description'].'" id="CheckboxGroup1_'.$skillrow['SkillID'].'" class="skillselect" value="yes" '.(($skillrow['SkillUserHas'] > 0) ? 'checked' : '').'>';
                            echo $skillrow['Description'].'</label>';
                            echo '<input type="number" name="'.$skillrow['Description'].'exp" class="expnumber" placeholder="Enter Experience in years." value="'.$skillrow['Experience'].'">';
                            echo '<br />';
                            echo '<br />';

                         } 
                         ?>
                        </div>
                        </p>
                </form>
        </div>
        <div class="Footer">
                <footer class="footer-basic-centered">
                        <p class="footer-company-motto">We Always Believe</p>
                        <p class="footer-links"> <a href="Home.php">Home</a> · <a href="Account.php">Account</a> · <a href="Projects.php">Projects</a> · <a href="Users.php">Users</a> · <a href="LogOut.php">LogOut</a> </p>
                        <p class="footer-company-name">Project Mainframe &copy; 2016</p>
                </footer>
        </div>
</div>
</body>
</html>
user2258597
  • 113
  • 1
  • 10
  • 5
    http://stackoverflow.com/questions/561066/fatal-error-allowed-memory-size-of-134217728-bytes-exhausted-codeigniter-xml probably its a duplicate of the given link.. – ameenulla0007 Feb 02 '16 at 12:15
  • @ameenulla0007 Yes, if you can, close vote it. `:)` – Praveen Kumar Purushothaman Feb 02 '16 at 12:17
  • This line `$current_skills[] = $skillrow;` is building a big array in memory.... arrays take a lot of memory..... why can't you simply process each record as you read it instead? – Mark Baker Feb 02 '16 at 12:18
  • Currently my query only has 3 rows to add to the array so it cant be that big surely? – user2258597 Feb 02 '16 at 12:22
  • How many skills can a user have? I would hope relatively few (ie, not in to the thousands) so I would hope there is not much storage required. But if there are lots then it will rapidly use up memory (and possibly suggest that you have a bug allowing numerous duplicate records to be inserted). No need to store it either, as you could loop around the result set directly to output the rows. You appear to have large amounts of repeated code in there as well. – Kickstart Feb 02 '16 at 12:24
  • Please refer this site https://www.airpair.com/php/fatal-error-allowed-memory-size, it might help. – dhi_m Feb 02 '16 at 12:25
  • @Kickstart A user can have upto 9 skills. The result why I tried using an `array` is to use the `in_array` function so I can find when a user has a skill and check the checkbox. Also the repeated code is for each different skill. I had previously posted on stack overflow asking for advice on creating a method to minimise repeated code, but had no responses (http://stackoverflow.com/questions/34946516/how-do-i-create-a-method-to-specify-field-name-and-a-pk-from-a-db-table) – user2258597 Feb 02 '16 at 12:29
  • But you can do that in a loop, removing the need for repeated code. Further your processing of the checks for $current_skills makes no sense (as you check the while array for a value, then use the SkillID array index, which is an index of a field within a row, rather than at the row level). – Kickstart Feb 02 '16 at 12:31
  • Reduce the looping by fetching the distinct data for the user. i.e) select distict(column) from table; – Karthik N Feb 02 '16 at 12:34
  • SkillID is a column in my database. Before I attempted to add the array I was just using `` but it was returning true for all as I couldn't specify which skill. – user2258597 Feb 02 '16 at 12:37
  • @Karthik N which looping are you refering to in your comment? – user2258597 Feb 02 '16 at 12:39
  • You have an endless loop as the mysqli_fetch_array function returns null rather than false once if hits the end of the result set. – Kickstart Feb 02 '16 at 12:49

2 Answers2

2

Quick attempt at cleaning the code up. This is just using a coded list of the skills rather than storing them on a table but hopefully will give you some ideas.

It could be made better if the skills table had the userid / skill id as a unique index, then you could just do an INSERT / on duplicate key update rather trying to read values to decide whether to insert or update a record

<?php 

error_reporting(E_ALL); ini_set('display_errors', 1); 
require 'Assets/Connections/Connections.php';
session_start(); 
    if(isset($_SESSION["UserID"]))
    {
    }
    else
    {
        header('Location: LogIn.php');
        die();
    }

    $User = (int)$_SESSION["UserID"];

    $result = $con->query("SELECT * FROM user WHERE UserID ='$User'") or die(mysqli_error($con));

    $row = $result->fetch_array(MYSQLI_BOTH);

    $_SESSION["FirstName"] = $row['Fname'];
    $_SESSION["LastName"] = $row['Lname'];
    $_SESSION["Email"] = $row['Email'];
    $_SESSION["Role"] = $row['JobRole'];

    $skills_array = array(1=>'Java',
                        7=>'iOS',
                        9=>'PHP',
                        3=>'SQL',
                        4=>'Windows',
                        5=>'Linux',
                        6=>'Unix',
                        8=>'Requirements Elicitation');

    if(isset($_POST['Update']))
    {

        $UpdateFName = $_SESSION["FirstName"];
        if ($_POST['FirstName'] != '' ) { $UpdateFName = $_POST['FirstName'];}
        $UpdateLName = $_SESSION["LastName"];
        if ($_POST['LastName'] != '' ) { $UpdateLName = $_POST['LastName'];}
        $UpdateEmail = $_SESSION["Email"];
        if ($_POST['Email'] != '' ) { $UpdateEmail = $_POST['Email'];}
        $UpdateRole = $_SESSION["Role"];
        if ($_POST['JobRole'] != '' ) { $UpdateRole = $_POST['JobRole'];}
        $PasswordCheck = $_POST['Password'];
        if(password_verify($PasswordCheck, $row['Password']))
        {

            $sql = $con->query("UPDATE user SET 
                Fname = '{$UpdateFName}', 
                Lname = '{$UpdateLName}', 
                Email = '{$UpdateEmail}', 
                JobRole = '{$UpdateRole}'
            WHERE UserID = $User") or die(mysqli_error($con));

            if(!empty($_FILES['file']['name']))
            {
                $file = basename($_FILES['file']['name']);
                move_uploaded_file($_FILES['file']['tmp_name'], 'Assets/Images/'.$file);
            }

            if(isset($file))
            {
                $sql = $con->query("UPDATE user SET ProfileImage = '".$_FILES['file']['name']."' WHERE UserID = $User") or die(mysqli_error($con));
            }

            $default = 0;

            foreach($skills_array AS $skills_id=>$skills_name)
            {
                if (isset($_POST[$skills_name]))
                {
                    if (empty($_POST[$skills_name.'exp']))
                    {
                        $exp = $default;
                    }
                    else
                    {
                        $exp = $_POST[$skills_name.'exp'];
                    }

                    $sql = $con->query("SELECT count(UserID) as total FROM userskills WHERE UserID = $User AND SkillID = ".$skills_id) or die(mysqli_error($con));

                    if ($row = mysqli_fetch_assoc($sql))
                    {
                        $sql = $con->query("INSERT INTO userskills ( UserID, SkillID, Experience) VALUES  ($User, $skills_id, $exp)");
                        //If the checkbox is not checked it will check to see if skill is already a skill assigned to the user. If they are it will delete it. If not it will ignore.   
                    }
                    else
                    {
                        $sql = $con->query("UPDATE userskills SET Experience = $exp WHERE UserID = $User AND SkillID $skills_id");
                    }
                } 
                else
                {
                    $sql = $con->query("DELETE FROM userskills WHERE UserID = $User AND SkillID = ".$skills_id);
                }
            }

            header('Location: Account.php');
            die();
        }
        else
        {
            echo 'Incorrect password please try again.';
        }
    }
?>
<!doctype html>
<html>
<head>
<link href="Assets/CSS/Master.css" rel="stylesheet" type="text/css" />
<link href="Assets/CSS/Menu.css" rel="stylesheet" type="text/css" />
<meta charset="utf-8">
<title>Update Account</title>
</head>

<body>
    <div class="Container">
        <div class="Header">
        </div>
        <div class="Menu">
            <div id="Menu">
                <nav>
                    <ul class="cssmenu">
                        <li><a href="Home.php">Home</a></li>
                        <li><a href="Account.php">Account</a></li>
                        <li><a href="Projects.php">Projects</a></li>
                        <li><a href="Users.php">Users</a></li>
                        <li><a href="LogOut.php">LogOut</a></li>
                    </ul>
                </nav>
            </div>
        </div>
        <div class="LeftBody"></div>
        <div class="RightBody">
            <form id="form1" name="form1" method="post" enctype="multipart/form-data">
                <div class="FormElement">
                    <input name="FirstName" type="text" class="TField" id="FirstName" placeholder="First Name" value="<?php echo $_SESSION["FirstName"]; ?>">
                </div>
                <div class="FormElement">
                    <input name="LastName" type="text" class="TField" id="LastName" placeholder="Last Name" value="<?php echo $_SESSION["LastName"]; ?>">
                </div>
                <div class="FormElement">
                    <input name="Email" type="email" class="TField" id="Email" placeholder="Email Address" value="<?php echo $_SESSION["Email"]; ?>">
                </div>
                <div class="FormElement">
                    <input name="JobRole" type="text" class="TField" id="JobRole" placeholder="Job Role" value="<?php echo $_SESSION["Role"]; ?>">
                </div>
                <div class="FormElement">
                    <input name="Password" type="password" class="TField" id="Password" placeholder="Password" required="requried">
                </div>
                <div class="FormElement">
                    <input type="file" name="file">
                <br />
                <br />
                </div>
                <p>
                    <?php

                        $result1 = $con->query("SELECT all_skills.SkillID, all_skills.SkillName, COUNT(userskills.SkillID) AS SkillKnown, MAX(Experience) AS Experience
                                                FROM 
                                                (
                                                    SELECT 1 AS SkillID, 'Java' AS SkillName
                                                    UNION
                                                    SELECT 7 AS SkillID, 'iOS' AS SkillName
                                                    UNION
                                                    SELECT 9 AS SkillID, 'PHP' AS SkillName
                                                    UNION
                                                    SELECT 3 AS SkillID, 'SQL' AS SkillName
                                                    UNION
                                                    SELECT 4 AS SkillID, 'Windows' AS SkillName
                                                    UNION
                                                    SELECT 5 AS SkillID, 'Linux' AS SkillName
                                                    UNION
                                                    SELECT 6 AS SkillID, 'Unix' AS SkillName
                                                    UNION
                                                    SELECT 8 AS SkillID, 'Requirements Elicitation'
                                                ) all_skills
                                                LEFT OUTER JOIN userskills 
                                                ON all_skills.SkillID = userskills.SkillID AND userskills.UserID = '$User' 
                                                GROUP BY all_skills.SkillID, all_skills.SkillName
                                                ORDER BY FIELD(all_skills.SkillID, 1, 7, 9, 3, 4, 5, 6, 8") or die(mysqli_error($con));

                        while ($skillrow = mysqli_fetch_array($result1, MYSQLI_ASSOC))
                        {
                            echo '<label>';
                            echo '<input type="checkbox" name="'.$skillrow['SkillName'].'" id="CheckboxGroup1_'.$skillrow['SkillID'].'" class="skillselect" value="yes" '.(($skillrow['SkillKnown'] > 0) ? 'checked' : '').'>';
                            echo $skillrow['SkillName'].'</label>';
                            echo '<input type="number" name="'.$skillrow['SkillName'].'exp" class="expnumber" placeholder="Enter Experience in years." value="'.$skillrow['Experience'].'">';
                            echo '<br />';
                            echo '<br />';
                        }

                    ?>
                </p>
                <div class="FormElement">
                    <input name="Update" type="submit" class="button" id="Update" value="Submit Changes">
                </div>
            </form>
        </div>
        <div class="Footer">
            <footer class="footer-basic-centered">
                <p class="footer-company-motto">We Always Believe</p>
                <p class="footer-links"> <a href="Home.php">Home</a> · <a href="Account.php">Account</a> · <a href="Projects.php">Projects</a> · <a href="Users.php">Users</a> · <a href="LogOut.php">LogOut</a> </p>
                <p class="footer-company-name">Project Mainframe &copy; 2016</p>
            </footer>
        </div>
    </div>
</body>
</html>
Kickstart
  • 21,403
  • 2
  • 21
  • 33
  • User ID and Skill ID are foreign keys in the userskills table. Can I do the "INSERT / on duplicate key update rather trying to read values to decide whether to insert or update a record"? Also see updated code above. – user2258597 Feb 02 '16 at 16:57
  • To use that you need a UNIQUE index covering both those columns (ie, one index that covers both columns). That will probably not cause you a problem. Nothing obvious wrong with the script now but will need testing (there are no doubt a few typos there from me!). – Kickstart Feb 02 '16 at 17:16
  • I really appreciate the time and effort you have put into helping me with this, top guy! I just have one more question for you... My skills are not being added to the table, but if I add them directly using a script they get removed. Which leads me to believe that there is maybe something wrong with the variable `$skills_name` ? – user2258597 Feb 02 '16 at 18:07
  • 2 minor problems I can see. Firstly at the top when you join the 3 tables together to get the users skills, you need to LEFT OUTER JOIN the user skills table (as you want a row for every possible skill for that user whether they have it or not, and then only the bit from userskills if the user has that skill). 2ndly you populate $skills_array as an array of arrays, but you then just treat it as an array when you loop through it. You probably should modify the last SELECT to use the skills table to avoid hard coding the skills. – Kickstart Feb 03 '16 at 09:18
0

Note that according to the docs, mysqli_fetch_array returns NULL if there are no further rows in the resultset. You are checking for false specifically rather than something that equates to false. So you have an endless loop. Change it to the following as a short term fix until you correct the other issues:-

while (($skillrow = mysqli_fetch_array($result1, MYSQLI_NUM)) != false)
{
    $current_skills[] = $skillrow;
}

Doing the rest of this as an answer although it is more of a comment but is too long.

Your code has a massive amount of repeated code.

For example you do pretty much the same code as below for each different skills:-

        //If the Unix checkbox is checked it will check to see if Unix is already a skill assigned to the user. If so it will ignore, if not it will add.   
    if (isset($_POST['unix'])){
        if (empty($_POST['unixexp'])){
            $unixexp = $default;
            }else{
            $unixexp = $_POST['unixexp'];}

        $sql = $con->query("SELECT count(UserID) as total FROM userskills WHERE UserID = $User AND SkillID = 6") 
        or die(mysqli_error($con));

        $row = mysqli_fetch_assoc($sql);

        if ($row ['total'] == "0"){

        $sql = $con->query("INSERT INTO userskills ( UserID, SkillID, Experience) VALUES  ($User, 6, $unixexp)");

    //If the Unix checkbox is not checked it will check to see if Unix is already a skill assigned to the user. If they are it will delete it. If not it will ignore.   
    }} else{

        $sql = $con->query("SELECT count(UserID) as total FROM userskills WHERE UserID = $User AND SkillID = 6") 
        or die(mysqli_error($con));

        $row = mysqli_fetch_assoc($sql);

        if ($row ['total'] == "1"){

        $sql = $con->query("DELETE FROM userskills 
        WHERE UserID = $User AND SkillID = 6");
        }}

This could easily be done by looping around an array of skills, or better looping around the results of a query of a table that stores those skills. This would make the code shorter, simpler and far easier to maintain (as in future you could just add the new skill to the array at the top, or better to the table of skills and not have to change the script itself at all).

Further if you had a table of skills you query of a users skills could be done as:-

SELECT a.SkillID, a.SkillName, COUNT(b.SkillID) AS SkillUserHas
FROM all_skills a
LEFT OUTER JOIN auserskills b
ON a.SkillID = b.SkillID
AND b.UserID = '$User'
GROUP BY a.SkillID, a.SkillName 

Then you can just loop around the results of this to output the list of checkboxes for the user to tick / untick the skills (this would return 1 row per skill, whether the user had it or not, and a column which would be 0 if they do not have that skill and >= 1 if that skill is recorded for them).

Note that you also appear to have done zero sanitation of variables. You need to use mysqli_real_escape_string or equivalent, otherwise the results could be nasty as users can indulge in SQL injection.

Kickstart
  • 21,403
  • 2
  • 21
  • 33