From 1f185ff5d470af75ac83fee8dad2f53f7e66903f Mon Sep 17 00:00:00 2001
From: Tim Hunt <T.J.Hunt@open.ac.uk>
Date: Wed, 12 Mar 2025 15:09:44 +0000
Subject: [PATCH] MDL-84846 core lock: report un-released locks better

We now report un-released locks in Behat tests, and when developer
debug is on, as well as in PHPunit.

The exception now has a full stack track, to help locate the problem.
---
 lib/classes/lock/lock.php | 13 +++++--------
 lib/tests/lock_test.php   | 27 ++++++++++++++++++++++++---
 2 files changed, 29 insertions(+), 11 deletions(-)

diff --git a/lib/classes/lock/lock.php b/lib/classes/lock/lock.php
index 44f78f60be6..3083a6e1fe0 100644
--- a/lib/classes/lock/lock.php
+++ b/lib/classes/lock/lock.php
@@ -52,12 +52,9 @@ class lock {
         $this->factory = $factory;
         $this->key = $key;
         $this->released = false;
-        $caller = debug_backtrace(true, 2)[1];
-        if ($caller && array_key_exists('file', $caller ) ) {
-            $this->caller = $caller['file'] . ' on line ' . $caller['line'];
-        } else if ($caller && array_key_exists('class', $caller)) {
-            $this->caller = $caller['class'] . $caller['type'] . $caller['function'];
-        }
+
+        // Track where the lock was raised, so we can report un-released locks in a helpful way.
+        $this->caller = format_backtrace(debug_backtrace(), true);
     }
 
     /**
@@ -120,7 +117,6 @@ class lock {
             if ($withexception) {
                 throw new \core\exception\coding_exception(<<<EOF
                     A lock was created but not released at: {$this->caller}
-
                     Code should look like:
 
                         \$factory = \core\lock\lock_config::get_lock_factory('type');
@@ -135,6 +131,7 @@ class lock {
      * Print debugging if this lock falls out of scope before being released.
      */
     public function __destruct() {
-        $this->release_if_not_released(defined('PHPUNIT_TEST'));
+        global $CFG;
+        $this->release_if_not_released(defined('BEHAT_SITE_RUNNING') || PHPUNIT_TEST || $CFG->debugdeveloper);
     }
 }
diff --git a/lib/tests/lock_test.php b/lib/tests/lock_test.php
index 50c85c7afbd..62148cbb7ee 100644
--- a/lib/tests/lock_test.php
+++ b/lib/tests/lock_test.php
@@ -16,6 +16,8 @@
 
 namespace core;
 
+use core\exception\coding_exception;
+
 /**
  * Unit tests for our locking implementations.
  *
@@ -23,6 +25,9 @@ namespace core;
  * @category   test
  * @copyright  2013 Damyon Wiese
  * @license    http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
+ * @covers \core\lock\db_record_lock_factory
+ * @covers \core\lock\file_lock_factory
+ * @covers \core\lock\lock
  */
 final class lock_test extends \advanced_testcase {
 
@@ -37,9 +42,9 @@ final class lock_test extends \advanced_testcase {
     /**
      * Run a suite of tests on a lock factory class.
      *
-     * @param class $lockfactoryclass - A lock factory class to test
+     * @param string $lockfactoryclass - name of a lock factory class to test.
      */
-    protected function run_on_lock_factory($lockfactoryclass) {
+    protected function run_on_lock_factory(string $lockfactoryclass): void {
 
         $modassignfactory = new $lockfactoryclass('mod_assign');
         $tooltaskfactory = new $lockfactoryclass('tool_task');
@@ -127,8 +132,24 @@ final class lock_test extends \advanced_testcase {
         // Manually create the core no-configuration factories.
         $this->run_on_lock_factory(\core\lock\db_record_lock_factory::class);
         $this->run_on_lock_factory(\core\lock\file_lock_factory::class);
-
     }
 
+    public function test_exception_for_unreleased_locks(): void {
+        $factory = new \core\lock\db_record_lock_factory('mod_assign');
+        $lock = $factory->get_lock('abc', 0);
+
+        // Using try/catch, not expectException, because I want to verify a few things.
+        try {
+            $lock->release_if_not_released(true);
+        } catch (coding_exception $e) {
+            $this->assertStringContainsString('A lock was created but not released at:', $e->getMessage());
+            $this->assertStringContainsString('call to core\lock\db_record_lock_factory->get_lock()', $e->getMessage());
+            $this->assertStringContainsString('call to core\lock_test->test_exception_for_unreleased_locks()', $e->getMessage());
+            return;
+        }
+
+        // Now use expectException to get the standard failure message.
+        $this->expectException(coding_exception::class);
+    }
 }