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

Scheduled task file_temp_cleanup_task throwing undefined index

    XMLWordPrintable

    Details

    • 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.

        --- a/lib/classes/task/file_temp_cleanup_task.php
        +++ b/lib/classes/task/file_temp_cleanup_task.php
        @@ -65,6 +65,7 @@ class file_temp_cleanup_task extends scheduled_task {
                     $modifieddateobject[$node] = $iter->getMTime();
                 }
         
        +        sleep(30);
                 // Now loop through again and remove old files and directories.
                 for ($iter->rewind(); $iter->valid(); $iter->next()) {
                     $node = $iter->getRealPath();
        
        

      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.

        $ touch /path/to/your/moodle/dataroot/temp/aloha.txt
        

      4. Verify that after the sleep has passed, the task ends without any PHP notice/warning
      5. Check that the file you touched (aloha.txt) remains in the temp directory.
      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. --- a/lib/classes/task/file_temp_cleanup_task.php +++ b/lib/classes/task/file_temp_cleanup_task.php @@ -65,6 +65,7 @@ class file_temp_cleanup_task extends scheduled_task { $modifieddateobject[$node] = $iter->getMTime(); } + sleep(30); // Now loop through again and remove old files and directories. for ($iter->rewind(); $iter->valid(); $iter->next()) { $node = $iter->getRealPath(); 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. $ touch /path/to/your/moodle/dataroot/temp/aloha.txt Verify that after the sleep has passed, the task ends without any PHP notice/warning Check that the file you touched (aloha.txt) remains in the temp directory.
    • Affected Branches:
      MOODLE_28_STABLE, MOODLE_30_STABLE, MOODLE_37_STABLE, MOODLE_39_STABLE
    • Fixed Branches:
      MOODLE_37_STABLE, MOODLE_38_STABLE
    • Pull from Repository:
    • Pull 3.7 Branch:
    • Pull 3.8 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

              Assignee:
              jonof Jonathon Fowler
              Reporter:
              colin Colin Campbell
              Peer reviewer:
              Simey Lameze
              Integrator:
              Eloy Lafuente (stronk7)
              Tester:
              Janelle Barcega
              Participants:
              Component watchers:
              Andrew Nicols, Jun Pataleta, Michael Hawkins, Shamim Rezaie, Simey Lameze
              Votes:
              3 Vote for this issue
              Watchers:
              7 Start watching this issue

                Dates

                Created:
                Updated:
                Resolved:
                Fix Release Date:
                9/Mar/20

                  Time Tracking

                  Estimated:
                  Original Estimate - 0 minutes
                  0m
                  Remaining:
                  Remaining Estimate - 0 minutes
                  0m
                  Logged:
                  Time Spent - 4 hours, 25 minutes
                  4h 25m