Added "unbound" parameter to CachedProperty as a workaround for circular references.

Entity.__hash__ now hashes its inthandle rather than its index (the later can be reused and match a new entity later on, while the former doesn't seems to be ever re-used from my testing).

Author: jordanbriere

addons/source-python/packages/source-python/entities/_base.py

@@ -169,7 +169,7 @@ def __init__(self, index, caching=True):
     def __hash__(self):
         """Return a hash value based on the entity index."""
         # Required for sets, because we have implemented __eq__
-        return hash(self.index)
+        return hash(self.inthandle)
 
     def __getattr__(self, attr):
         """Find if the attribute is valid and returns the appropriate value."""

addons/source-python/packages/source-python/entities/helpers.py

@@ -5,10 +5,6 @@
 # =============================================================================
 # >> IMPORTS
 # =============================================================================
-# Python Imports
-#   WeakRef
-from weakref import proxy
-
 # Source.Python Imports
 #   Core
 from core.cache import CachedProperty
@@ -101,11 +97,7 @@ def __init__(self, wrapped_self, wrapper):
         func = wrapped_self.__getattr__(wrapper.__name__)
         super().__init__(func._manager, func._type_name, func, func._this)
         self.wrapper = wrapper
-
-        # Don't store a strong reference to the wrapped instance.
-        # If we do, we will ends with a circular reference preventing itself,
-        # along with everything it refers, to ever be garbage collected.
-        self.wrapped_self = proxy(wrapped_self)
+        self.wrapped_self = wrapped_self
 
     def __call__(self, *args, **kwargs):
         return super().__call__(
@@ -128,5 +120,6 @@ def wrap_entity_mem_func(wrapper):
 
     return CachedProperty(
         lambda self: EntityMemFuncWrapper(self, wrapper),
-        doc=wrapper.__doc__
+        doc=wrapper.__doc__,
+        unbound=True
     )

src/core/modules/core/core_cache.cpp

@@ -36,13 +36,14 @@
 //-----------------------------------------------------------------------------
 CCachedProperty::CCachedProperty(
 	object fget=object(), object fset=object(), object fdel=object(), const char *doc=NULL,
-	boost::python::tuple args=boost::python::tuple(), dict kwargs=dict())
+	bool unbound=false, boost::python::tuple args=boost::python::tuple(), dict kwargs=dict())
 {
 	set_getter(fget);
 	set_setter(fset);
 	set_deleter(fdel);
 
 	m_szDocString = doc;
+	m_bUnbound = unbound;
 
 	m_args = args;
 	m_kwargs = kwargs;
@@ -88,6 +89,66 @@ object CCachedProperty::_prepare_value(object value)
 	return values;
 }
 
+void CCachedProperty::_invalidate_cache(PyObject *pRef)
+{
+	try
+	{
+		m_cache[handle<>(pRef)].del();
+	}
+	catch (...)
+	{
+		if (!PyErr_ExceptionMatches(PyExc_KeyError))
+			throw_error_already_set();
+
+		PyErr_Clear();
+	}
+}
+
+void CCachedProperty::_update_cache(object instance, object value)
+{
+	if (m_bUnbound)
+		m_cache[handle<>(
+			PyWeakref_NewRef(
+				instance.ptr(),
+				make_function(
+					boost::bind(&CCachedProperty::_invalidate_cache, this, _1),
+					default_call_policies(),
+					boost::mpl::vector2<void, PyObject *>()
+				).ptr()
+			)
+		)] = value;
+	else
+	{
+		dict cache = extract<dict>(instance.attr("__dict__"));
+		cache[m_name] = value;
+	}
+}
+
+void CCachedProperty::_delete_cache(object instance)
+{
+	try
+	{
+		if (m_bUnbound)
+			m_cache[
+				handle<>(
+					PyWeakref_NewRef(instance.ptr(), NULL)
+				)
+			].del();
+		else
+		{
+			dict cache = extract<dict>(instance.attr("__dict__"));
+			cache[m_name].del();
+		}
+	}
+	catch (...)
+	{
+		if (!PyErr_ExceptionMatches(PyExc_KeyError))
+			throw_error_already_set();
+
+		PyErr_Clear();
+	}
+}
+
 
 object CCachedProperty::get_getter()
 {
@@ -149,11 +210,21 @@ object CCachedProperty::__get__(object self, object instance, object owner=objec
 			"Unable to retrieve the value of an unbound property."
 		);
 
-	object cache = extract<dict>(instance.attr("__dict__"));
+	object value;
 
 	try
 	{
-		return cache[name];
+		if (pSelf.m_bUnbound)
+			return pSelf.m_cache[
+				handle<>(
+					PyWeakref_NewRef(instance.ptr(), NULL)
+				)
+			];
+		else
+		{
+			dict cache = extract<dict>(instance.attr("__dict__"));
+			return cache[name];
+		}
 	}
 	catch (...)
 	{
@@ -169,15 +240,17 @@ object CCachedProperty::__get__(object self, object instance, object owner=objec
 				"Unable to retrieve the value of a property that have no getter function."
 			);
 
-		cache[name] = pSelf._prepare_value(
+		value = pSelf._prepare_value(
 			getter(
 				*(make_tuple(handle<>(borrowed(instance.ptr()))) + pSelf.m_args),
 				**pSelf.m_kwargs
 			)
 		);
+
+		pSelf._update_cache(instance, value);
 	}
 
-	return cache[name];
+	return value;
 }
 
 
@@ -194,20 +267,10 @@ void CCachedProperty::__set__(object instance, object value)
 		**m_kwargs
 	);
 
-	dict cache = extract<dict>(instance.attr("__dict__"));
 	if (!result.is_none())
-		cache[m_name] = _prepare_value(result);
+		_update_cache(instance, _prepare_value(result));
 	else
-	{
-		try
-		{
-			cache[m_name].del();
-		}
-		catch (...)
-		{
-			PyErr_Clear();
-		}
-	}
+		_delete_cache(instance);
 }
 
 void CCachedProperty::__delete__(object instance)
@@ -218,15 +281,7 @@ void CCachedProperty::__delete__(object instance)
 			**m_kwargs
 		);
 
-	dict cache = extract<dict>(instance.attr("__dict__"));
-	try
-	{
-		cache[m_name].del();
-	}
-	catch (...)
-	{
-		PyErr_Clear();
-	}
+	_delete_cache(instance);
 }
 
 object CCachedProperty::__call__(object self, object fget)

src/core/modules/core/core_cache.h

@@ -40,10 +40,16 @@ using namespace boost::python;
 class CCachedProperty
 {
 public:
-	CCachedProperty(object fget, object fset, object fdel, const char *doc, boost::python::tuple args, dict kwargs);
+	CCachedProperty(
+		object fget, object fset, object fdel, const char *doc, bool unbound,
+		boost::python::tuple args, dict kwargs
+	);
 
 	static object _callable_check(object function, const char *szName);
 	static object _prepare_value(object value);
+	void _invalidate_cache(PyObject *pRef);
+	void _update_cache(object instance, object value);
+	void _delete_cache(object instance);
 
 	object get_getter();
 	object set_getter(object fget);
@@ -73,6 +79,9 @@ class CCachedProperty
 
 	str m_name;
 
+	bool m_bUnbound;
+	dict m_cache;
+
 public:
 	const char *m_szDocString;
 

src/core/modules/core/core_cache_wrap.cpp

@@ -54,10 +54,10 @@ void export_cached_property(scope _cache)
 {
 	class_<CCachedProperty, CCachedProperty *> CachedProperty(
 		"CachedProperty",
-		init<object, object, object, const char *, boost::python::tuple, dict>(
+		init<object, object, object, const char *, bool, boost::python::tuple, dict>(
 			(
 				arg("fget")=object(), arg("fset")=object(), arg("fdel")=object(), arg("doc")=object(),
-				arg("args")=boost::python::tuple(), arg("kwargs")=dict()
+				arg("unbound")=false, arg("args")=boost::python::tuple(), arg("kwargs")=dict()
 			),
 			"Represents a property attribute that is only"
 			" computed once and cached.\n"
@@ -83,6 +83,10 @@ void export_cached_property(scope _cache)
 			"	Deleter signature: self, *args, **kwargs\n"
 			":param str doc:\n"
 			"	Documentation string for this property.\n"
+			":param bool unbound\n"
+			"	Whether the cached objects should be independently maintained rather than bound to"
+			" the instance they belong to. The cache will be slightly slower to lookup, but this can"
+			" be required in certain cases to prevent circular references/memory leak.\n"
 			":param tuple args:\n"
 			"	Extra arguments passed to the getter, setter and deleter functions.\n"
 			":param dict kwargs:\n"