MDL-54205 backup: loggers close() and destroy()

Any backup & restore operation may be leaving opened files
if a file logger is being used. This implementes the close()
method, so every logger can close any resource.

Also, the recommended backup_controlled::destroy() method
now calls to new logger::destroy() method in charge of
deleting all the references and closing any resource.

Finally, some internally used controllers, were missing
their destroy call, leading to associated loggers to
remain open. Now all them are explicitly deltroyed.

backup/controller/backup_controller.class.php

@@ -158,6 +158,8 @@ public function __construct($type, $id, $format, $interactive, $mode, $userid){
     public function destroy() {
         // Only need to destroy circulars under the plan. Delegate to it.
         $this->plan->destroy();
+        // Loggers may have also chained references, destroy them. Also closing resources when needed.
+        $this->logger->destroy();
     }
 
     public function finish_ui() {
@@ -184,7 +186,7 @@ public function set_status($status) {
             $this->save_controller();
             $tbc = self::load_controller($this->backupid);
             $this->logger = $tbc->logger; // wakeup loggers
-            $tbc->destroy(); // Clean temp controller structures
+            $tbc->plan->destroy(); // Clean plan controller structures, keeping logger alive.
 
         } else if ($status == backup::STATUS_FINISHED_OK) {
             // If the operation has ended without error (backup::STATUS_FINISHED_OK)

backup/controller/restore_controller.class.php

@@ -170,6 +170,8 @@ public function __construct($tempdir, $courseid, $interactive, $mode, $userid, $
     public function destroy() {
         // Only need to destroy circulars under the plan. Delegate to it.
         $this->plan->destroy();
+        // Loggers may have also chained references, destroy them. Also closing resources when needed.
+        $this->logger->destroy();
     }
 
     public function finish_ui() {
@@ -196,7 +198,7 @@ public function set_status($status) {
             $this->save_controller();
             $tbc = self::load_controller($this->restoreid);
             $this->logger = $tbc->logger; // wakeup loggers
-            $tbc->destroy(); // Clean temp controller structures
+            $tbc->plan->destroy(); // Clean plan controller structures, keeping logger alive.
 
         } else if ($status == backup::STATUS_FINISHED_OK) {
             // If the operation has ended without error (backup::STATUS_FINISHED_OK)

backup/upgrade.txt

@@ -1,6 +1,14 @@
 This files describes API changes in /backup/*,
 information provided here is intended especially for developers.
 
+=== 3.1 ===
+
+* New close() method added to loggers so they can close any open resource. Previously
+  any backup and restore operation using the file logger may be leaving unclosed files.
+* New destroy() method added to loggers, normally called from backup and restore controllers
+  own destroy() method to ensure that all references in the chained loggers are deleted
+  and any open resource within them is closed properly.
+
 === 3.0 ===
 
 * The backup_auto_keep setting, in automated backups configuration, is now

backup/util/dbops/restore_dbops.class.php

@@ -101,9 +101,11 @@ public static function get_included_tasks($restoreid) {
 
             // If included, add it
             if ($included) {
-                $includedtasks[] = $task;
+                $includedtasks[] = clone($task); // A clone is enough. In fact we only need the basepath.
             }
         }
+        $rc->destroy(); // Always need to destroy.
+
         return $includedtasks;
     }
 
@@ -1510,8 +1512,12 @@ public static function precheck_included_users($restoreid, $courseid, $userid, $
         // Calculate the context we are going to use for capability checking
         $context = context_course::instance($courseid);
 
+        // TODO: Some day we must kill this dependency and change the process
+        // to pass info around without loading a controller copy.
         // When conflicting users are detected we may need original site info.
-        $restoreinfo = restore_controller_dbops::load_controller($restoreid)->get_info();
+        $rc = restore_controller_dbops::load_controller($restoreid);
+        $restoreinfo = $rc->get_info();
+        $rc->destroy(); // Always need to destroy.
 
         // Calculate if we have perms to create users, by checking:
         // to 'moodle/restore:createuser' and 'moodle/restore:userinfo'

backup/util/helper/backup_helper.class.php

@@ -302,6 +302,7 @@ static public function store_backup_file($backupid, $filepath, \core\progress\ba
                     $bc = backup_controller::load_controller($backupid);
                     $bc->log('Attempt to copy backup file to the specified directory using filesystem failed - ',
                             backup::LOG_WARNING, $dir);
+                    $bc->destroy();
                 }
                 // bad luck, try to deal with the file the old way - keep backup in file area if we can not copy to ext system
             }

backup/util/loggers/base_logger.class.php

@@ -71,6 +71,30 @@ public function get_level() {
         return $this->level;
     }
 
+    /**
+     * Destroy (nullify) the chain of loggers references, also closing resources when needed.
+     *
+     * @since Moodle 3.1
+     */
+    public final function destroy() {
+        // Recursively destroy the chain.
+        if ($this->next !== null) {
+            $this->next->destroy();
+            $this->next = null;
+        }
+        // And close every logger.
+        $this->close();
+    }
+
+    /**
+     * Close any resource the logger may have open.
+     *
+     * @since Moodle 3.1
+     */
+    public function close() {
+        // Nothing to do by default. Only loggers using resources (files, own connections...) need to override this.
+    }
+
 // checksumable interface methods
 
     public function calculate_checksum() {

backup/util/loggers/file_logger.class.php

@@ -66,6 +66,19 @@ public function __wakeup() {
         }
     }
 
+    /**
+     * Close the logger resources (file handle) if still open.
+     *
+     * @since Moodle 3.1
+     */
+    public function close() {
+        // Close the file handle if hasn't been closed already.
+        if (is_resource($this->fhandle)) {
+            fclose($this->fhandle);
+            $this->fhandle = null;
+        }
+    }
+
 // Protected API starts here
 
     protected function action($message, $level, $options = null) {