Make sure same hook spot is not updated while executing

atk4 · Jul 16, 2023 · 9c83378 · 9c83378
1 parent ecac6c6
commit 9c83378
Showing 1 changed file with 23 additions and 9 deletions.
diff --git a/src/HookTrait.php b/src/HookTrait.php
@@ -6,11 +6,7 @@
 
 trait HookTrait
 {
-    /**
-     * Contains information about configured hooks (callbacks).
-     *
-     * @var array<string, array<int, array<int, array{\Closure, 1?: array<int, mixed>}>>>
-     */
+    /** @var array<string, array<int, array<int, array{\Closure, 1?: array<int, mixed>}>>> Configured hooks (callbacks). */
     protected array $hooks = [];
 
     /** Next hook index counter. */
@@ -19,6 +15,9 @@ trait HookTrait
     /** @var \WeakReference<static>|null */
     private ?\WeakReference $_hookOrigThis = null;
 
+    /** @var string[] */
+    private $_hookActiveSpots = [];
+
     /**
      * Optimize GC. When a Closure is guaranteed to be rebound before invoke, it can be rebound
      * to (deduplicated) fake instance before safely.
@@ -107,6 +106,15 @@ private function _rebindHooksIfCloned(): void
         $this->_hookOrigThis = \WeakReference::create($this);
     }
 
+    private function _hookRequireInactive(string $spot): void
+    {
+        if ($this->_hookActiveSpots[$spot] ?? false) {
+            throw (new Exception('Hook spot must be inactive for requested operation, but it is already executing'))
+                ->addMoreInfo('object', $this)
+                ->addMoreInfo('spot', $spot);
+        }
+    }
+
     /**
      * Add another callback to be executed during hook($spot);.
      *
@@ -121,6 +129,7 @@ private function _rebindHooksIfCloned(): void
     public function onHook(string $spot, \Closure $fx, array $args = [], int $priority = 5): int
     {
         $this->_rebindHooksIfCloned();
+        $this->_hookRequireInactive($spot);
 
         $fx = $this->_unbindHookFxIfBoundToThis($fx, false);
 
@@ -244,6 +253,8 @@ public function onHookDynamicShort(string $spot, \Closure $getFxThisFx, \Closure
      */
     public function removeHook(string $spot, int $priority = null, bool $priorityIsIndex = false)
     {
+        $this->_hookRequireInactive($spot);
+
         if ($priority !== null) {
             if ($priorityIsIndex) {
                 $index = $priority;
@@ -303,13 +314,16 @@ public function hook(string $spot, array $args = [], HookBreaker &$brokenBy = nu
     {
         $brokenBy = null;
         $this->_rebindHooksIfCloned();
+        $this->_hookRequireInactive($spot);
 
         $return = [];
         if (isset($this->hooks[$spot])) {
-            krsort($this->hooks[$spot]); // lower priority is called sooner
-            $hooksBackup = $this->hooks[$spot];
+            ksort($this->hooks[$spot]); // lower priority is called sooner
+
             try {
-                while ($hooks = array_pop($this->hooks[$spot])) {
+                $this->_hookActiveSpots[$spot] = true;
+
+                foreach ($this->hooks[$spot] as $hooks) {
                     foreach ($hooks as $index => [$hookFx, $hookArgs]) {
                         $return[$index] = $hookFx($this, ...$args, ...$hookArgs);
                     }
@@ -319,7 +333,7 @@ public function hook(string $spot, array $args = [], HookBreaker &$brokenBy = nu
 
                 return $e->getReturnValue();
             } finally {
-                $this->hooks[$spot] = $hooksBackup;
+                $this->_hookActiveSpots[$spot] = false;
             }
         }