Remove dependency on weak reference for registry callbacks

The use of weakref was introduced as a preventive measure to avoid
potential OOM kills, however that limited our ability to employ
certain functions as callbacks, such as object methods (see [1] for
an example).

Since the adoption of the callback registry, it has been observed that
callbacks are generally long lived (for the entire duration of the
process they belong to), therefore this limitation appears to be too
restrictive at this point in time.

Some might argue that it's better safe than sorry, but until we
have some evidence of actual OOM kills, it's probably best to take
the bolder action of removing the adoption of weak references and
deal with the potential fallout, should it happen.

[1] https://review.openstack.org/#/c/175179/

Change-Id: Idcd0286fc4235af82901c8a17ea45bc758b62b37

diff --git a/doc/source/devref/callbacks.rst b/doc/source/devref/callbacks.rst
index 3b7a043b36fe9d2e930e8a3a8eba72f3af206415..4c9f54884475bdc124980949720fe392e45b6339 100644 (file)
--- a/doc/source/devref/callbacks.rst
+++ b/doc/source/devref/callbacks.rst
@@ -326,9 +326,55 @@ Is the registry thread-safe?
   In this case, chances that things do go badly may be pretty slim. Making the registry
   thread-safe will be considered as a future improvement.
 
-Can I use lambdas or 'closures' as callbacks?
+What kind of function can be a callback?
 
-  Currently a weakref.proxy(callback) is registered instead of the callback itself. This means
-  that certain constructs like lambdas, cannot be used as callbacks. Even though this limitation
-  could be easily lifted, use of methods or module functions should be preferred over lambdas
-  or nested functions for maintanability and testability reasons.
+  Anything you fancy: lambdas, 'closures', class, object or module methods. For instance:
+
+::
+
+  from neutron.callbacks import events
+  from neutron.callbacks import resources
+  from neutron.callbacks import registry
+
+
+  def callback1(resource, event, trigger, **kwargs):
+      print 'module callback'
+
+
+  class MyCallback(object):
+
+      def callback2(self, resource, event, trigger, **kwargs):
+          print 'object callback'
+
+      @classmethod
+      def callback3(cls, resource, event, trigger, **kwargs):
+          print 'class callback'
+
+
+  c = MyCallback()
+  registry.subscribe(callback1, resources.ROUTER, events.BEFORE_CREATE)
+  registry.subscribe(c.callback2, resources.ROUTER, events.BEFORE_CREATE)
+  registry.subscribe(MyCallback.callback3, resources.ROUTER, events.BEFORE_CREATE)
+
+  def do_notify():
+      def nested_subscribe(resource, event, trigger, **kwargs):
+          print 'nested callback'
+
+      registry.subscribe(nested_subscribe, resources.ROUTER, events.BEFORE_CREATE)
+
+      kwargs = {'foo': 'bar'}
+      registry.notify(resources.ROUTER, events.BEFORE_CREATE, do_notify, **kwargs)
+
+
+  print 'Notifying...'
+  do_notify()
+
+And the output is going to be:
+
+::
+
+  Notifying...
+  module callback
+  object callback
+  class callback
+  nested callback

diff --git a/neutron/callbacks/manager.py b/neutron/callbacks/manager.py
index 365a54f27f37e005992f46febcf03b9c96f2deb8..d1f3c95d14dd6698450a06742370d16ab547b2d9 100644 (file)
--- a/neutron/callbacks/manager.py
+++ b/neutron/callbacks/manager.py
@@ -11,7 +11,6 @@
 #    under the License.
 
 import collections
-import weakref
 
 from oslo_log import log as logging
 from oslo_utils import reflection
@@ -47,7 +46,7 @@ class CallbacksManager(object):
             raise exceptions.Invalid(element='event', value=event)
 
         callback_id = _get_id(callback)
-        self._callbacks[resource][event][callback_id] = weakref.proxy(callback)
+        self._callbacks[resource][event][callback_id] = callback
         # We keep a copy of callbacks to speed the unsubscribe operation.
         if callback_id not in self._index:
             self._index[callback_id] = collections.defaultdict(set)