Skip to content

Commit

Permalink
Remove Weakref usage
Browse files Browse the repository at this point in the history
- While it initially worked, usage has grown problematic as we've done more
  performance optimizations, and as Weakref itself has changed. What I found was
  that Weakref and the GC were cleaning up early, making listeners disappear -
  and thus making tests fail unexpectedly. Since we never want a listener to
  disappear, it makes sense to remove this functionality.
  • Loading branch information
weierophinney committed May 7, 2013
1 parent 19ad7e9 commit b7e2abf
Show file tree
Hide file tree
Showing 5 changed files with 2 additions and 105 deletions.
1 change: 0 additions & 1 deletion composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@
"doctrine/annotations": "Doctrine Annotations >=1.0 for annotation features",
"ircmaxell/random-lib": "Fallback random byte generator for Zend\\Math\\Rand if OpenSSL/Mcrypt extensions are unavailable",
"ocramius/proxy-manager": "ProxyManager to handle lazy initialization of services",
"pecl-weakref": "Implementation of weak references for Zend\\Stdlib\\CallbackHandler",
"zendframework/zendpdf": "ZendPdf for creating PDF representations of barcodes",
"zendframework/zendservice-recaptcha": "ZendService\\ReCaptcha for rendering ReCaptchas in Zend\\Captcha and/or Zend\\Form"
},
Expand Down
4 changes: 0 additions & 4 deletions library/Zend/EventManager/EventManager.php
Original file line number Diff line number Diff line change
Expand Up @@ -462,10 +462,6 @@ protected function triggerListeners($event, EventInterface $e, $callback = null)

foreach ($listeners as $listener) {
$listenerCallback = $listener->getCallback();
if (!$listenerCallback) {
$this->detach($listener);
continue;
}

// Trigger the listener's callback, and push its result onto the
// response collection
Expand Down
85 changes: 2 additions & 83 deletions library/Zend/Stdlib/CallbackHandler.php
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@

use Closure;
use ReflectionClass;
use WeakRef;

/**
* CallbackHandler
Expand Down Expand Up @@ -39,12 +38,6 @@ class CallbackHandler
*/
protected static $isPhp54;

/**
* Is pecl/weakref extension installed?
* @var bool
*/
protected static $hasWeakRefExtension;

/**
* Constructor
*
Expand All @@ -60,13 +53,6 @@ public function __construct($callback, array $metadata = array())
/**
* Registers the callback provided in the constructor
*
* If you have pecl/weakref {@see http://pecl.php.net/weakref} installed,
* this method provides additional behavior.
*
* If a callback is a functor, or an array callback composing an object
* instance, this method will pass the object to a WeakRef instance prior
* to registering the callback.
*
* @param callable $callback
* @throws Exception\InvalidCallbackException
* @return void
Expand All @@ -77,44 +63,7 @@ protected function registerCallback($callback)
throw new Exception\InvalidCallbackException('Invalid callback provided; not callable');
}

if (null === static::$hasWeakRefExtension) {
static::$hasWeakRefExtension = class_exists('WeakRef');
}

// If pecl/weakref is not installed, simply store the callback and return
if (!static::$hasWeakRefExtension) {
$this->callback = $callback;
return;
}

// If WeakRef exists, we want to use it.

// If we have a non-closure object, pass it to WeakRef, and then
// register it.
if (is_object($callback) && !$callback instanceof Closure) {
$this->callback = new WeakRef($callback);
return;
}

// If we have a string or closure, register as-is
if (!is_array($callback)) {
$this->callback = $callback;
return;
}

list($target, $method) = $callback;

// If we have an array callback, and the first argument is not an
// object, register as-is
if (!is_object($target)) {
$this->callback = $callback;
return;
}

// We have an array callback with an object as the first argument;
// pass it to WeakRef, and then register the new callback
$target = new WeakRef($target);
$this->callback = array($target, $method);
$this->callback = $callback;
}

/**
Expand All @@ -124,32 +73,7 @@ protected function registerCallback($callback)
*/
public function getCallback()
{
$callback = $this->callback;

// String callbacks -- simply return
if (is_string($callback)) {
return $callback;
}

// WeakRef callbacks -- pull it out of the object and return it
if ($callback instanceof WeakRef) {
return $callback->get();
}

// Non-WeakRef object callback -- return it
if (is_object($callback)) {
return $callback;
}

// Array callback with WeakRef object -- retrieve the object first, and
// then return
list($target, $method) = $callback;
if ($target instanceof WeakRef) {
return array($target->get(), $method);
}

// Otherwise, return it
return $callback;
return $this->callback;
}

/**
Expand All @@ -162,11 +86,6 @@ public function call(array $args = array())
{
$callback = $this->getCallback();

// WeakRef object will return null if the real object was disposed
if (null === $callback) {
return null;
}

// Minor performance tweak, if the callback gets called more than once
if (!isset(static::$isPhp54)) {
static::$isPhp54 = version_compare(PHP_VERSION, '5.4.0rc1', '>=');
Expand Down
1 change: 0 additions & 1 deletion library/Zend/Stdlib/composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@
},
"target-dir": "Zend/Stdlib",
"suggest": {
"pecl-weakref": "Implementation of weak references for Stdlib\\CallbackHandler",
"zendframework/zend-servicemanager": "To support hydrator plugin manager usage"
},
"require": {
Expand Down
16 changes: 0 additions & 16 deletions tests/ZendTest/EventManager/EventManagerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -546,22 +546,6 @@ public function testTriggerCanTakeAnOptionalCallbackArgumentToEmulateTriggerUnti
$this->assertTrue($responses->stopped());
}

public function testWeakRefsAreHonoredWhenTriggering()
{
if (!class_exists('WeakRef', false)) {
$this->markTestSkipped('Requires pecl/weakref');
}

$functor = new TestAsset\Functor;
$this->events->attach('test', $functor);

unset($functor);

$result = $this->events->trigger('test', $this, array());
$message = $result->last();
$this->assertNull($message);
}

public function testDuplicateIdentifiersAreNotRegistered()
{
$events = new EventManager(array(__CLASS__, get_class($this)));
Expand Down

0 comments on commit b7e2abf

Please sign in to comment.