Merge branch 'MDL-67667-main' of https://github.com/andrewnicols/moodle

admin/cli/scheduled_task.php

@@ -189,11 +189,7 @@
     }
 
     $task->set_lock($lock);
-    if (!$task->is_blocking()) {
-        $cronlock->release();
-    } else {
-        $task->set_cron_lock($cronlock);
-    }
+    $cronlock->release();
 
     \core\cron::run_inner_scheduled_task($task);
 }

auth/ldap/classes/task/asynchronous_sync_task.php

@@ -34,15 +34,13 @@
  * @license    http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
  */
 class asynchronous_sync_task extends adhoc_task {
-
     /** @var string Message prefix for mtrace */
     protected const MTRACE_MSG = 'Synced ldap users';
 
     /**
      * Constructor
      */
     public function __construct() {
-        $this->set_blocking(false);
         $this->set_component('auth_ldap');
     }
 

backup/backup.php

@@ -203,7 +203,6 @@
 
             // Create adhoc task for backup.
             $asynctask = new \core\task\asynchronous_backup_task();
-            $asynctask->set_blocking(false);
             $asynctask->set_custom_data(array('backupid' => $backupid));
             $asynctask->set_userid($USER->id);
             \core\task\manager::queue_adhoc_task($asynctask);

backup/restore.php

@@ -173,7 +173,6 @@
     // Create adhoc task for restore.
     $restoreid = $restore->get_restoreid();
     $asynctask = new \core\task\asynchronous_restore_task();
-    $asynctask->set_blocking(false);
     $asynctask->set_userid($USER->id);
     $asynctask->set_custom_data(array('backupid' => $restoreid));
     \core\task\manager::queue_adhoc_task($asynctask);

backup/tests/async_backup_test.php

@@ -105,7 +105,6 @@ public function test_async_backup() {
 
         // Create the adhoc task.
         $asynctask = new \core\task\asynchronous_backup_task();
-        $asynctask->set_blocking(false);
         $asynctask->set_custom_data(['backupid' => $backupid]);
         $asynctask->set_userid($USER->id);
         \core\task\manager::queue_adhoc_task($asynctask);
@@ -151,7 +150,6 @@ public function test_async_backup() {
 
         // Create the adhoc task.
         $asynctask = new \core\task\asynchronous_backup_task();
-        $asynctask->set_blocking(false);
         $asynctask->set_custom_data(['backupid' => $backupid]);
         \core\task\manager::queue_adhoc_task($asynctask);
 
@@ -242,7 +240,6 @@ public function test_complete_async_backup() {
 
         // Now queue an adhoc task and check it handles and completes gracefully.
         $asynctask = new \core\task\asynchronous_backup_task();
-        $asynctask->set_blocking(false);
         $asynctask->set_custom_data(array('backupid' => $backupid));
         \core\task\manager::queue_adhoc_task($asynctask);
 

backup/tests/async_restore_test.php

@@ -116,7 +116,6 @@ public function test_async_restore() {
 
         // Create the adhoc task.
         $asynctask = new \core\task\asynchronous_restore_task();
-        $asynctask->set_blocking(false);
         $asynctask->set_custom_data(array('backupid' => $restoreid));
         $asynctask->set_userid($USER->id);
         \core\task\manager::queue_adhoc_task($asynctask);
@@ -223,7 +222,6 @@ public function test_async_restore_missing_controller() {
 
         // Create the adhoc task.
         $asynctask = new \core\task\asynchronous_restore_task();
-        $asynctask->set_blocking(false);
         $asynctask->set_custom_data(['backupid' => $restoreid]);
         \core\task\manager::queue_adhoc_task($asynctask);
 
@@ -245,7 +243,6 @@ public function test_async_restore_missing_controller() {
 
         // Create the adhoc task.
         $asynctask = new \core\task\asynchronous_restore_task();
-        $asynctask->set_blocking(false);
         $asynctask->set_custom_data(['backupid' => $restoreid]);
         \core\task\manager::queue_adhoc_task($asynctask);
 

backup/util/helper/backup_cron_helper.class.php

@@ -378,7 +378,6 @@ private static function push_course_backup_adhoc_task($backupcourse, $admin) {
         global $DB;
 
         $asynctask = new \core\task\course_backup_task();
-        $asynctask->set_blocking(false);
         $asynctask->set_custom_data(array(
             'courseid' => $backupcourse->courseid,
             'adminid' => $admin->id

backup/util/helper/copy_helper.class.php

@@ -96,7 +96,6 @@ public static function create_copy(\stdClass $copydata): array {
 
         // Create the ad-hoc task to perform the course copy.
         $asynctask = new \core\task\asynchronous_copy_task();
-        $asynctask->set_blocking(false);
         $asynctask->set_custom_data($copyids);
         \core\task\manager::queue_adhoc_task($asynctask);
 

backup/util/helper/tests/copy_helper_test.php

@@ -333,7 +333,6 @@ public function test_create_copy() {
 
         $this->assertInstanceOf('\\core\\task\\asynchronous_copy_task', $task);
         $this->assertEquals($result, (array)$task->get_custom_data());
-        $this->assertFalse($task->is_blocking());
 
         \core\task\manager::adhoc_task_complete($task);
     }

cache/stores/file/tests/asyncpurge_test.php

@@ -86,7 +86,6 @@ public function test_cache_async_purge_cron() {
 
         // Create / execute adhoc task to delete cache revision directory.
         $asynctask = new cachestore_file\task\asyncpurge();
-        $asynctask->set_blocking(false);
         $asynctask->set_custom_data(['path' => $cacherevdir]);
         $asynctask->set_userid($USER->id);
         \core\task\manager::queue_adhoc_task($asynctask);

course/tests/backup/restore_test.php

@@ -159,7 +159,6 @@ protected function async_restore_course($backupid, $courseid, $userid, $target)
 
         // Create the adhoc task.
         $asynctask = new \core\task\asynchronous_restore_task();
-        $asynctask->set_blocking(false);
         $asynctask->set_custom_data(array('backupid' => $restoreid));
         \core\task\manager::queue_adhoc_task($asynctask);
 

lib/classes/task/manager.php

@@ -304,7 +304,6 @@ public static function record_from_scheduled_task($task) {
         $record = new \stdClass();
         $record->classname = self::get_canonical_class_name($task);
         $record->component = $task->get_component();
-        $record->blocking = $task->is_blocking();
         $record->customised = $task->is_customised();
         $record->lastruntime = $task->get_last_run_time();
         $record->nextruntime = $task->get_next_run_time();
@@ -333,7 +332,6 @@ public static function record_from_adhoc_task($task) {
         $record->classname = self::get_canonical_class_name($task);
         $record->id = $task->get_id();
         $record->component = $task->get_component();
-        $record->blocking = $task->is_blocking();
         $record->nextruntime = $task->get_next_run_time();
         $record->faildelay = $task->get_fail_delay();
         $record->customdata = $task->get_custom_data_as_string();
@@ -368,7 +366,6 @@ public static function adhoc_task_from_record($record) {
         if (isset($record->component)) {
             $task->set_component($record->component);
         }
-        $task->set_blocking(!empty($record->blocking));
         if (isset($record->faildelay)) {
             $task->set_fail_delay($record->faildelay);
         }
@@ -430,7 +427,6 @@ public static function scheduled_task_from_record($record, $expandr = true, $ove
         if (isset($record->component)) {
             $task->set_component($record->component);
         }
-        $task->set_blocking(!empty($record->blocking));
         if (isset($record->minute)) {
             $task->set_minute($record->minute, $expandr);
         }
@@ -1040,11 +1036,7 @@ private static function set_locks(adhoc_task $task, lock $lock, lock_factory $cr
         }
 
         $task->set_lock($lock);
-        if (!$task->is_blocking()) {
-            $cronlock->release();
-        } else {
-            $task->set_cron_lock($cronlock);
-        }
+        $cronlock->release();
     }
 
     /**
@@ -1107,11 +1099,7 @@ public static function get_next_scheduled_task($timestart) {
                     throw new \moodle_exception('locktimeout');
                 }
 
-                if (!$task->is_blocking()) {
-                    $cronlock->release();
-                } else {
-                    $task->set_cron_lock($cronlock);
-                }
+                $cronlock->release();
                 return $task;
             }
         }
@@ -1198,9 +1186,6 @@ public static function adhoc_task_failed(adhoc_task $task) {
         $DB->update_record('task_adhoc', $record);
 
         $task->release_concurrency_lock();
-        if ($task->is_blocking()) {
-            $task->get_cron_lock()->release();
-        }
         $task->get_lock()->release();
 
         self::$runningtask = null;
@@ -1259,9 +1244,6 @@ public static function adhoc_task_complete(adhoc_task $task) {
 
         // Release the locks.
         $task->release_concurrency_lock();
-        if ($task->is_blocking()) {
-            $task->get_cron_lock()->release();
-        }
         $task->get_lock()->release();
 
         self::$runningtask = null;
@@ -1311,9 +1293,6 @@ public static function scheduled_task_failed(scheduled_task $task) {
         $record->pid = null;
         $DB->update_record('task_scheduled', $record);
 
-        if ($task->is_blocking()) {
-            $task->get_cron_lock()->release();
-        }
         $task->get_lock()->release();
 
         self::$runningtask = null;
@@ -1394,9 +1373,6 @@ public static function scheduled_task_complete(scheduled_task $task) {
         }
 
         // Reschedule and then release the locks.
-        if ($task->is_blocking()) {
-            $task->get_cron_lock()->release();
-        }
         $task->get_lock()->release();
 
         self::$runningtask = null;

lib/classes/task/task_base.php

@@ -45,9 +45,6 @@ abstract class task_base {
     /** @var string $component - The component this task belongs to. */
     private $component = '';
 
-    /** @var bool $blocking - Does this task block the entire cron process. */
-    private $blocking = false;
-
     /** @var int $faildelay - Exponentially increasing fail delay */
     private $faildelay = 0;
 
@@ -120,18 +117,38 @@ public function get_cron_lock() {
 
     /**
      * Setter for $blocking.
-     * @param bool $blocking
+     *
+     * Please note that task blocking is no longer supported.
+     * If you are using it in older versions of Moodle you are strongly advised to rewrite your code
+     * as has a detrimental impact upon performance.
+     *
+     * @deprecated since Moodle 4.4 See MDL-67667
+     * @todo Remove in MDL-81509
      */
+    #[\core\attribute\deprecated(
+        replacement: null,
+        since: '4.4',
+        reason: 'Blocking tasks are no longer supported',
+    )]
     public function set_blocking($blocking) {
-        $this->blocking = $blocking;
+        \core\deprecation::emit_deprecation_if_present([$this, __FUNCTION__]);
     }
 
     /**
      * Getter for $blocking.
+     *
      * @return bool
+     * @deprecated since Moodle 4.4 See MDL-67667
+     * @todo Remove in MDL-81509
      */
+    #[\core\attribute\deprecated(
+        replacement: null,
+        since: '4.4',
+        reason: 'Blocking tasks are no longer supported',
+    )]
     public function is_blocking() {
-        return $this->blocking;
+        \core\deprecation::emit_deprecation_if_present([$this, __FUNCTION__]);
+        return false;
     }
 
     /**

lib/db/install.xml

@@ -1,5 +1,5 @@
 <?xml version="1.0" encoding="UTF-8" ?>
-<XMLDB PATH="lib/db" VERSION="20240326" COMMENT="XMLDB file for core Moodle tables"
+<XMLDB PATH="lib/db" VERSION="20240408" COMMENT="XMLDB file for core Moodle tables"
     xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
     xsi:noNamespaceSchemaLocation="../../lib/xmldb/xmldb.xsd"
 >
@@ -3475,7 +3475,6 @@
         <FIELD NAME="classname" TYPE="char" LENGTH="255" NOTNULL="true" SEQUENCE="false" COMMENT="The class extending scheduled_task to be called when running this task."/>
         <FIELD NAME="lastruntime" TYPE="int" LENGTH="10" NOTNULL="false" SEQUENCE="false"/>
         <FIELD NAME="nextruntime" TYPE="int" LENGTH="10" NOTNULL="false" SEQUENCE="false"/>
-        <FIELD NAME="blocking" TYPE="int" LENGTH="2" NOTNULL="true" DEFAULT="0" SEQUENCE="false" COMMENT="Block the entire cron when this task is running."/>
         <FIELD NAME="minute" TYPE="char" LENGTH="200" NOTNULL="true" SEQUENCE="false"/>
         <FIELD NAME="hour" TYPE="char" LENGTH="70" NOTNULL="true" SEQUENCE="false"/>
         <FIELD NAME="day" TYPE="char" LENGTH="90" NOTNULL="true" SEQUENCE="false"/>
@@ -3504,7 +3503,6 @@
         <FIELD NAME="faildelay" TYPE="int" LENGTH="10" NOTNULL="false" SEQUENCE="false"/>
         <FIELD NAME="customdata" TYPE="text" NOTNULL="false" SEQUENCE="false" COMMENT="Custom data to be passed to the adhoc task. Must be serialisable using json_encode()"/>
         <FIELD NAME="userid" TYPE="int" LENGTH="10" NOTNULL="false" SEQUENCE="false"/>
-        <FIELD NAME="blocking" TYPE="int" LENGTH="2" NOTNULL="true" DEFAULT="0" SEQUENCE="false"/>
         <FIELD NAME="timecreated" TYPE="int" LENGTH="10" NOTNULL="true" DEFAULT="0" SEQUENCE="false" COMMENT="Timestamp of adhoc task creation"/>
         <FIELD NAME="timestarted" TYPE="int" LENGTH="10" NOTNULL="false" SEQUENCE="false" COMMENT="Time when the task was started"/>
         <FIELD NAME="hostname" TYPE="char" LENGTH="255" NOTNULL="false" SEQUENCE="false" COMMENT="Hostname where the task is running"/>

lib/db/upgrade.php

@@ -1144,5 +1144,28 @@ function xmldb_main_upgrade($oldversion) {
         upgrade_main_savepoint(true, 2024032600.01);
     }
 
+    if ($oldversion < 2024041200.00) {
+        // Define field blocking to be dropped from task_adhoc.
+        $table = new xmldb_table('task_adhoc');
+        $field = new xmldb_field('blocking');
+
+        // Conditionally launch drop field blocking.
+        if ($dbman->field_exists($table, $field)) {
+            $dbman->drop_field($table, $field);
+        }
+
+        // Define field blocking to be dropped from task_scheduled.
+        $table = new xmldb_table('task_scheduled');
+        $field = new xmldb_field('blocking');
+
+        // Conditionally launch drop field blocking.
+        if ($dbman->field_exists($table, $field)) {
+            $dbman->drop_field($table, $field);
+        }
+
+        // Main savepoint reached.
+        upgrade_main_savepoint(true, 2024041200.00);
+    }
+
     return true;
 }

lib/phpunit/classes/advanced_testcase.php

@@ -705,11 +705,7 @@ protected function runAdhocTasks($matchclass = '', $matchuserid = null) {
             }
 
             $task->set_lock($lock);
-            if (!$task->is_blocking()) {
-                $cronlock->release();
-            } else {
-                $task->set_cron_lock($cronlock);
-            }
+            $cronlock->release();
 
             \core\cron::prepare_core_renderer();
             \core\cron::setup_user($user);

lib/tests/behat/behat_general.php

@@ -1176,11 +1176,7 @@ public function i_run_the_scheduled_task($taskname) {
             throw new DriverException('Unable to obtain task lock for scheduled task');
         }
         $task->set_lock($lock);
-        if (!$task->is_blocking()) {
-            $cronlock->release();
-        } else {
-            $task->set_cron_lock($cronlock);
-        }
+        $cronlock->release();
 
         try {
             // Prepare the renderer.

lib/upgrade.txt

@@ -35,6 +35,9 @@ information provided here is intended especially for developers.
       By default, tasks will be retried until they succeed, other tasks can override this method to change this behaviour.
     - set_attempts_available(): Used to set the number of attempts available for the task
     - get_attempts_available(): Used to get the number of attempts available for the task.
+* Support for blocking tasks has been removed. See MDL-67667 for further information.
+  Please note that this feature never worked correctly and can cause serious performance issues.
+  It is also not possible to deprecate this feature in a notifiable way.
 * There is a new DML method $DB->get_fieldset. For some reason, this did not exist even though get_fieldset_select etc. did.
 * The following callbacks have been migrated to hooks:
   - before_standard_html_head() -> core\hook\output\before_standard_head_html_generation

version.php

@@ -29,7 +29,7 @@
 
 defined('MOODLE_INTERNAL') || die();
 
-$version  = 2024040900.00;              // YYYYMMDD      = weekly release date of this DEV branch.
+$version  = 2024041200.00;              // YYYYMMDD      = weekly release date of this DEV branch.
                                         //         RR    = release increments - 00 in DEV branches.
                                         //           .XX = incremental changes.
 $release  = '4.4dev+ (Build: 20240409)'; // Human-friendly version name