Uploaded image for project: 'Moodle'
  1. Moodle
  2. MDL-52189

Scheduled task file_temp_cleanup_task throwing undefined index

    XMLWordPrintable

    Details

    • Type: Bug
    • Status: Waiting for peer review
    • Priority: Minor
    • Resolution: Unresolved
    • Affects Version/s: 2.8.9, 3.0, 3.7.2
    • Fix Version/s: None
    • Component/s: Administration
    • Labels:
    • Testing Instructions:
      Hide
      1. Manually execute the 'core\task\file_temp_cleanup_task' task. Ensure no unusual error or warning messages are reported.
      2. Modify lib/classes/task/file_temp_cleanup_task.php to add a sleep(30); statement between the two 'for' loops of the execute_on() function.
      3. Execute the 'core\task\file_temp_cleanup_task' task again, and in the 30 seconds it is sleeping, touch a new file in the Moodle 'temp' directory. After the sleep has passed and the task ends, observe that the file you touched remains.
      Show
      Manually execute the 'core\task\file_temp_cleanup_task' task. Ensure no unusual error or warning messages are reported. Modify lib/classes/task/file_temp_cleanup_task.php to add a sleep(30); statement between the two 'for' loops of the execute_on() function. Execute the 'core\task\file_temp_cleanup_task' task again, and in the 30 seconds it is sleeping, touch a new file in the Moodle 'temp' directory. After the sleep has passed and the task ends, observe that the file you touched remains.
    • Affected Branches:
      MOODLE_28_STABLE, MOODLE_30_STABLE, MOODLE_37_STABLE
    • Pull from Repository:
    • Pull 3.6 Branch:
    • Pull 3.7 Branch:
    • Pull Master Branch:

      Description

      The cron file_temp_cleanup_task class is routinely creating large cron error output containing messages like this:

      PHP Notice: Undefined index: /my/path/to/moodledata/temp/assignfeedback_editpdf/src-4106e701b71870d2d89dccc472a6a337412a15ad.pdf in /my/path/to/moodle/lib/classes/task/file_temp_cleanup_task.php on line 76

      This occurs because the second $iter->rewind causes directory contents to be listed that were not listed after the first $iter->rewind. (See the below 2.8.9 code from lib/classes/task/file_temp_cleanup_task.php.)

      We should be able to fix this by changing

       71             if (!is_readable($node)) {
       72                 continue;
       73             }
      

      to something like this

       71             if (!isset($modifieddateobject[$node]) or !is_readable($node)) {
       72                 continue;
       73             }
      

      The original execute function for the task in lib/classes/task/file_temp_cleanup_task.php:

       44     public function execute() {
       45         global $CFG;
       46 
       47         $tmpdir = $CFG->tempdir;
       48         // Default to last weeks time.
       49         $time = strtotime('-1 week');
       50 
       51         $dir = new \RecursiveDirectoryIterator($tmpdir);
       52         // Show all child nodes prior to their parent.
       53         $iter = new \RecursiveIteratorIterator($dir, \RecursiveIteratorIterator::CHILD_FIRST);
       54 
       55         // An array of the full path (key) and date last modified.
       56         $modifieddateobject = array();
       57 
       58         // Get the time modified for each directory node. Nodes will be updated
       59         // once a file is deleted, so we need a list of the original values.
       60         for ($iter->rewind(); $iter->valid(); $iter->next()) {
       61             $node = $iter->getRealPath();
       62             if (!is_readable($node)) {
       63                 continue;
       64             }
       65             $modifieddateobject[$node] = $iter->getMTime();
       66         }
       67 
       68         // Now loop through again and remove old files and directories.
       69         for ($iter->rewind(); $iter->valid(); $iter->next()) {
       70             $node = $iter->getRealPath();
       71             if (!is_readable($node)) {
       72                 continue;
       73             }
       74 
       75             // Check if file or directory is older than the given time.
       76             if ($modifieddateobject[$node] < $time) {
       77                 if ($iter->isDir() && !$iter->isDot()) {
       78                     // Don't attempt to delete the directory if it isn't empty.
       79                     if (!glob($node. DIRECTORY_SEPARATOR . '*')) {
       80                         if (@rmdir($node) === false) {
       81                             mtrace("Failed removing directory '$node'.");
       82                         }
       83                     }
       84                 }
       85                 if ($iter->isFile()) {
       86                     if (@unlink($node) === false) {
       87                         mtrace("Failed removing file '$node'.");
       88                     }
       89                 }
       90             } else {
       91                 // Return the time modified to the original date only for real files.
       92                 if ($iter->isDir() && !$iter->isDot()) {
       93                     touch($node, $modifieddateobject[$node]);
       94                 }
       95             }
       96         }
       97     }
      

      The code history indicates that this likely started with the fix for MDL-48252.

        Attachments

          Issue Links

            Activity

              People

              • Votes:
                3 Vote for this issue
                Watchers:
                4 Start watching this issue

                Dates

                • Created:
                  Updated: