From db62ebed4955883fc08a425556152848fc1ed8e9 Mon Sep 17 00:00:00 2001 From: Marc Jauvin Date: Sun, 20 Feb 2022 12:40:05 -0500 Subject: [PATCH 1/6] Fix problem when an event listener defined as a closure has variable references --- src/Events/Dispatcher.php | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/Events/Dispatcher.php b/src/Events/Dispatcher.php index db5e9b934..58b3cbd29 100644 --- a/src/Events/Dispatcher.php +++ b/src/Events/Dispatcher.php @@ -48,10 +48,10 @@ public function listen($events, $listener = null, $priority = 0) } elseif ($listener instanceof QueuedClosure) { $listener = $listener->resolve(); } - $listener = Serialization::wrapClosure($listener); foreach ((array) $events as $event) { if (Str::contains($event, '*')) { + $listener = Serialization::wrapClosure($listener); $this->setupWildcardListen($event, $listener); } else { $this->listeners[$event][$priority][] = $this->makeListener($listener); @@ -61,6 +61,14 @@ public function listen($events, $listener = null, $priority = 0) } } + // Serialize the new listener + public function makeListener($listener, $wildcard = false) + { + $listener = parent::makeListener($listener, $wildcard); + + return Serialization::wrapClosure($listener); + } + /** * Get the event that is currently firing. * From 08888c005495f98745d8d06aa334963c1a6dbedf Mon Sep 17 00:00:00 2001 From: Marc Jauvin Date: Sun, 20 Feb 2022 12:44:59 -0500 Subject: [PATCH 2/6] no need to get the closure, the wrapped closure will automatically reference the internal closure when called --- src/Events/Dispatcher.php | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/Events/Dispatcher.php b/src/Events/Dispatcher.php index 58b3cbd29..013d212fc 100644 --- a/src/Events/Dispatcher.php +++ b/src/Events/Dispatcher.php @@ -135,9 +135,6 @@ public function dispatch($event, $payload = [], $halt = false) } foreach ($this->getListeners($event) as $listener) { - if ($listener instanceof SerializableClosure) { - $listener = $listener->getClosure(); - } $response = $listener($event, $payload); // If a response is returned from the listener and event halting is enabled From 948ad959c6a2f1b24d93ab468347ffb1f2b5186b Mon Sep 17 00:00:00 2001 From: Marc Jauvin Date: Sun, 20 Feb 2022 13:03:45 -0500 Subject: [PATCH 3/6] dummy commit to force the tests --- src/Events/Dispatcher.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Events/Dispatcher.php b/src/Events/Dispatcher.php index 013d212fc..685005583 100644 --- a/src/Events/Dispatcher.php +++ b/src/Events/Dispatcher.php @@ -61,7 +61,7 @@ public function listen($events, $listener = null, $priority = 0) } } - // Serialize the new listener + // Serialize the listener created by laravel public function makeListener($listener, $wildcard = false) { $listener = parent::makeListener($listener, $wildcard); From cfd8fab02bcb9f553e4becb5a7fead62337cba00 Mon Sep 17 00:00:00 2001 From: Marc Jauvin Date: Sun, 20 Feb 2022 15:28:26 -0500 Subject: [PATCH 4/6] make sure variable value gets modified by event handler when variable gets passed by reference --- tests/Events/DispatcherTest.php | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/tests/Events/DispatcherTest.php b/tests/Events/DispatcherTest.php index c48005a42..5a6a425b1 100644 --- a/tests/Events/DispatcherTest.php +++ b/tests/Events/DispatcherTest.php @@ -53,6 +53,19 @@ public function testTypedClosureListen() $this->assertTrue($magic_value); } + public function testClosureWithReferenceArgument() + { + $original = false; + + $dispatcher = new Dispatcher(); + $dispatcher->listen('test', function (&$value) { + $value = true; + }); + $dispatcher->dispatch('test', [&$original]); + + $this->assertTrue($original); + } + public function testStringEventPriorities() { $magic_value = 0; From 1c6cb1949ee3ae6ef933a255fe5a7014955ebf9f Mon Sep 17 00:00:00 2001 From: Marc Jauvin Date: Sun, 20 Feb 2022 15:34:29 -0500 Subject: [PATCH 5/6] add test for variable passed by value --- tests/Events/DispatcherTest.php | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/tests/Events/DispatcherTest.php b/tests/Events/DispatcherTest.php index 5a6a425b1..2cf08f419 100644 --- a/tests/Events/DispatcherTest.php +++ b/tests/Events/DispatcherTest.php @@ -53,6 +53,19 @@ public function testTypedClosureListen() $this->assertTrue($magic_value); } + public function testClosureWithValueArgument() + { + $original = false; + + $dispatcher = new Dispatcher(); + $dispatcher->listen('test', function ($value) { + $value = true; + }); + $dispatcher->dispatch('test', [$original]); + + $this->assertFalse($original); + } + public function testClosureWithReferenceArgument() { $original = false; From 277a6a58142398179011a58eda951acbcec3c5b5 Mon Sep 17 00:00:00 2001 From: Marc Jauvin Date: Tue, 22 Feb 2022 08:00:55 -0500 Subject: [PATCH 6/6] simplify code a bit --- src/Events/Dispatcher.php | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/Events/Dispatcher.php b/src/Events/Dispatcher.php index 685005583..d67c46f67 100644 --- a/src/Events/Dispatcher.php +++ b/src/Events/Dispatcher.php @@ -51,8 +51,7 @@ public function listen($events, $listener = null, $priority = 0) foreach ((array) $events as $event) { if (Str::contains($event, '*')) { - $listener = Serialization::wrapClosure($listener); - $this->setupWildcardListen($event, $listener); + $this->setupWildcardListen($event, Serialization::wrapClosure($listener)); } else { $this->listeners[$event][$priority][] = $this->makeListener($listener);