Merge "Use weakrefs for common_db_mixin callbacks"

neutron/common/utils.py

@@ -27,6 +27,7 @@ import signal
 import sys
 import time
 import uuid
+import weakref
 
 import debtcollector
 from debtcollector import removals
@@ -879,3 +880,17 @@ def get_related_rand_names(prefixes, max_length=None):
 def get_related_rand_device_names(prefixes):
     return get_related_rand_names(prefixes,
                                   max_length=n_const.DEVICE_NAME_MAX_LEN)
+
+
+try:
+    # PY3
+    weak_method = weakref.WeakMethod
+except AttributeError:
+    # PY2
+    import weakrefmethod
+    weak_method = weakrefmethod.WeakMethod
+
+
+def make_weak_ref(f):
+    """Make a weak reference to a function accounting for bound methods."""
+    return weak_method(f) if hasattr(f, '__self__') else weakref.ref(f)

neutron/db/common_db_mixin.py

@@ -24,6 +24,7 @@ from sqlalchemy import or_
 from sqlalchemy import sql
 
 from neutron.api.v2 import attributes
+from neutron.common import utils
 from neutron.db import _utils as ndb_utils
 
 
@@ -65,12 +66,20 @@ class CommonDbMixin(object):
         Filter hooks take as input the filter expression being built and return
         a transformed filter expression
         """
+        if callable(query_hook):
+            query_hook = utils.make_weak_ref(query_hook)
+        if callable(filter_hook):
+            filter_hook = utils.make_weak_ref(filter_hook)
+        if callable(result_filters):
+            result_filters = utils.make_weak_ref(result_filters)
         cls._model_query_hooks.setdefault(model, {})[name] = {
             'query': query_hook, 'filter': filter_hook,
             'result_filters': result_filters}
 
     @classmethod
     def register_dict_extend_funcs(cls, resource, funcs):
+        funcs = [utils.make_weak_ref(f) if callable(f) else f
+                 for f in funcs]
         cls._dict_extend_functions.setdefault(resource, []).extend(funcs)
 
     @property
@@ -108,15 +117,11 @@ class CommonDbMixin(object):
         # Execute query hooks registered from mixins and plugins
         for _name, hooks in six.iteritems(self._model_query_hooks.get(model,
                                                                       {})):
-            query_hook = hooks.get('query')
-            if isinstance(query_hook, six.string_types):
-                query_hook = getattr(self, query_hook, None)
+            query_hook = self._resolve_ref(hooks.get('query'))
             if query_hook:
                 query = query_hook(context, model, query)
 
-            filter_hook = hooks.get('filter')
-            if isinstance(filter_hook, six.string_types):
-                filter_hook = getattr(self, filter_hook, None)
+            filter_hook = self._resolve_ref(hooks.get('filter'))
             if filter_hook:
                 query_filter = filter_hook(context, model, query_filter)
 
@@ -192,24 +197,29 @@ class CommonDbMixin(object):
                     query = query.filter(is_shared)
             for _nam, hooks in six.iteritems(self._model_query_hooks.get(model,
                                                                          {})):
-                result_filter = hooks.get('result_filters', None)
-                if isinstance(result_filter, six.string_types):
-                    result_filter = getattr(self, result_filter, None)
-
+                result_filter = self._resolve_ref(
+                    hooks.get('result_filters', None))
                 if result_filter:
                     query = result_filter(query, filters)
         return query
 
+    def _resolve_ref(self, ref):
+        """Finds string ref functions, handles dereference of weakref."""
+        if isinstance(ref, six.string_types):
+            ref = getattr(self, ref, None)
+        if isinstance(ref, weakref.ref):
+            ref = ref()
+        return ref
+
     def _apply_dict_extend_functions(self, resource_type,
                                      response, db_object):
         for func in self._dict_extend_functions.get(
             resource_type, []):
             args = (response, db_object)
-            if isinstance(func, six.string_types):
-                func = getattr(self, func, None)
-            else:
+            if not isinstance(func, six.string_types):
                 # must call unbound method - use self as 1st argument
                 args = (self,) + args
+            func = self._resolve_ref(func)
             if func:
                 func(*args)
 

neutron/services/tag/tag_plugin.py

@@ -118,11 +118,16 @@ class TagPlugin(common_db_mixin.CommonDbMixin, tag_ext.TagPluginBase):
             except exc.NoResultFound:
                 raise tag_ext.TagNotFound(tag=tag)
 
-    # support only _apply_dict_extend_functions supported resources
-    # at the moment.
-    for resource, model in resource_model_map.items():
-        common_db_mixin.CommonDbMixin.register_dict_extend_funcs(
-            resource, [_extend_tags_dict])
-        common_db_mixin.CommonDbMixin.register_model_query_hook(
-            model, "tag", None, None,
-            functools.partial(tag_methods.apply_tag_filters, model))
+    def __new__(cls, *args, **kwargs):
+        inst = super(TagPlugin, cls).__new__(cls, *args, **kwargs)
+        inst._filter_methods = []  # prevent GC of our partial functions
+        # support only _apply_dict_extend_functions supported resources
+        # at the moment.
+        for resource, model in resource_model_map.items():
+            common_db_mixin.CommonDbMixin.register_dict_extend_funcs(
+                resource, [_extend_tags_dict])
+            method = functools.partial(tag_methods.apply_tag_filters, model)
+            inst._filter_methods.append(method)
+            common_db_mixin.CommonDbMixin.register_model_query_hook(
+                model, "tag", None, None, method)
+        return inst

neutron/tests/tools.py

@@ -125,7 +125,7 @@ class CommonDbMixinHooksFixture(fixtures.Fixture):
     def _setUp(self):
         self.original_hooks = common_db_mixin.CommonDbMixin._model_query_hooks
         self.addCleanup(self.restore_hooks)
-        common_db_mixin.CommonDbMixin._model_query_hooks = copy.deepcopy(
+        common_db_mixin.CommonDbMixin._model_query_hooks = copy.copy(
             common_db_mixin.CommonDbMixin._model_query_hooks)
 
     def restore_hooks(self):

requirements.txt

@@ -46,6 +46,7 @@ oslo.versionedobjects>=1.17.0 # Apache-2.0
 osprofiler>=1.4.0 # Apache-2.0
 ovs>=2.6.1 # Apache-2.0
 pyroute2>=0.4.12 # Apache-2.0 (+ dual licensed GPL2)
+weakrefmethod>=1.0.2;python_version=='2.7'  # PSF
 
 python-novaclient!=2.33.0,>=2.29.0 # Apache-2.0
 python-designateclient>=1.5.0 # Apache-2.0