Fixed CachedProperty's decorators overriding the main descriptor name.

Removed CachedGenerator as caching it was leaking a shallow copy of the entire frame. The generated values are now computed and cached on retrieval, rather than iteration.
Improved the logic of CachedProperty.__get__.
Removed some redundant cast/extract between CPython and Boost.
Fixed some exceptions being silenced under certain conditions within the load/unload process.

Author: jordanbriere

addons/source-python/packages/source-python/core/cache.py

@@ -12,7 +12,6 @@
 # =============================================================================
 # Source.Python Imports
 #   Core
-from _core._cache import CachedGenerator
 from _core._cache import CachedProperty
 from _core._cache import cached_property
 
@@ -21,7 +20,6 @@
 # >> ALL DECLARATION
 # =============================================================================
 __all__ = [
-    'CachedGenerator',
     'CachedProperty',
     'cached_property'
 ]

src/core/modules/core/core_cache.cpp

@@ -62,9 +62,30 @@ object CCachedProperty::_callable_check(object function, const char *szName)
 
 object CCachedProperty::_prepare_value(object value)
 {
-	if (PyGen_Check(value.ptr()))
-		value = object(CCachedGenerator(value));
-	return value;
+	if (!PyGen_Check(value.ptr()))
+		return value;
+
+	if (getattr(value, "gi_frame").is_none())
+		BOOST_RAISE_EXCEPTION(
+			PyExc_ValueError,
+			"The given generator is exhausted."
+		);
+
+	list values;
+	while (true)
+	{
+		try
+		{
+			values.append(value.attr("__next__")());
+		}
+		catch(...)
+		{
+			PyErr_Clear();
+			break;
+		}
+	}
+
+	return values;
 }
 
 
@@ -76,7 +97,7 @@ object CCachedProperty::get_getter()
 object CCachedProperty::set_getter(object fget)
 {
 	m_fget = _callable_check(fget, "getter");
-	return object(ptr(this));
+	return fget;
 }
 
 
@@ -88,7 +109,7 @@ object CCachedProperty::get_setter()
 object CCachedProperty::set_setter(object fset)
 {
 	m_fset = _callable_check(fset, "setter");
-	return object(ptr(this));
+	return fset;
 }
 
 
@@ -100,7 +121,7 @@ object CCachedProperty::get_deleter()
 object CCachedProperty::set_deleter(object fdel)
 {
 	m_fdel = _callable_check(fdel, "deleter");
-	return object(ptr(this));
+	return fdel;
 }
 
 
@@ -121,44 +142,43 @@ void CCachedProperty::__set_name__(object owner, str name)
 	m_name = name;
 }
 
-
 object CCachedProperty::__get__(object instance, object owner=object())
 {
 	if (instance.is_none())
 		return object(ptr(this));
 
-	if (!m_name)
+	if (m_name.is_none())
 		BOOST_RAISE_EXCEPTION(
 			PyExc_AttributeError,
 			"Unable to retrieve the value of an unbound property."
 		);
 
-	PyObject *pCache = PyObject_GetAttrString(instance.ptr(), "__dict__");
+	object cache = extract<dict>(instance.attr("__dict__"));
 
-	if (!PyDict_Check(pCache))
-		BOOST_RAISE_EXCEPTION(
-			PyExc_ValueError,
-			"Cache dictionary is invalid."
-		);
-
-	Py_DECREF(pCache);
-	PyObject *pValue = PyDict_GetItemString(pCache, extract<const char *>(m_name));
-	if (pValue)
-		return object(handle<>(borrowed(pValue)));
-
-	if (m_fget.is_none())
-		BOOST_RAISE_EXCEPTION(
-			PyExc_AttributeError,
-			"Unable to retrieve the value of a property that have no getter function."
+	try
+	{
+		return cache[m_name];
+	}
+	catch (...)
+	{
+		if (!PyErr_ExceptionMatches(PyExc_KeyError))
+			throw_error_already_set();
+
+		PyErr_Clear();
+
+		if (m_fget.is_none())
+			BOOST_RAISE_EXCEPTION(
+				PyExc_AttributeError,
+				"Unable to retrieve the value of a property that have no getter function."
+			);
+
+		cache[m_name] = _prepare_value(
+			m_fget(
+				*(make_tuple(handle<>(borrowed(instance.ptr()))) + m_args),
+				**m_kwargs
+			)
 		);
-
-	dict cache = extract<dict>(pCache);
-	cache[m_name] = _prepare_value(
-		m_fget(
-			*(make_tuple(handle<>(borrowed(instance.ptr()))) + m_args),
-			**m_kwargs
-		)
-	);
+	}
 
 	return cache[m_name];
 }
@@ -181,7 +201,7 @@ void CCachedProperty::__set__(object instance, object value)
 	if (!result.is_none())
 		cache[m_name] = _prepare_value(result);
 	else
-		PyDict_DelItemString(cache.ptr(), extract<const char *>(m_name));
+		cache[m_name].del();
 }
 
 void CCachedProperty::__delete__(object instance)
@@ -193,7 +213,13 @@ void CCachedProperty::__delete__(object instance)
 		);
 
 	dict cache = extract<dict>(instance.attr("__dict__"));
-	PyDict_DelItemString(cache.ptr(), extract<const char *>(m_name));
+	cache[m_name].del();
+}
+
+object CCachedProperty::__call__(object fget)
+{
+	m_fget = _callable_check(fget, "getter");
+	return object(ptr(this));
 }
 
 object CCachedProperty::__getitem__(str item)
@@ -218,49 +244,3 @@ CCachedProperty *CCachedProperty::wrap_descriptor(object descriptor, object owne
 
 	return pProperty;
 }
-
-
-//-----------------------------------------------------------------------------
-// CCachedGenerator class.
-//-----------------------------------------------------------------------------
-CCachedGenerator::CCachedGenerator(object generator)
-{
-	if (!PyGen_Check(generator.ptr()))
-		BOOST_RAISE_EXCEPTION(
-			PyExc_TypeError,
-			"The given generator is invalid."
-		);
-
-	object frame = generator.attr("gi_frame");
-	if (frame.is_none())
-		BOOST_RAISE_EXCEPTION(
-			PyExc_ValueError,
-			"The given generator is exhausted."
-		);
-
-	m_generator = generator;
-}
-
-
-object CCachedGenerator::get_generator()
-{
-	return m_generator;
-}
-
-
-object CCachedGenerator::__iter__()
-{
-	while(!m_generator.is_none())
-	{
-		try
-		{
-			m_generated_values.append(m_generator.attr("__next__")());
-		}
-		catch(...)
-		{
-			m_generator = object();
-			PyErr_Clear();
-		}
-	}
-	return m_generated_values.attr("__iter__")();
-}

src/core/modules/core/core_cache.h

@@ -61,6 +61,7 @@ class CCachedProperty
 	object __get__(object instance, object owner);
 	void __set__(object instance, object value);
 	void __delete__(object instance);
+	object __call__(object fget);
 	object __getitem__(str item);
 	void __setitem__(str item, object value);
 
@@ -82,22 +83,4 @@ class CCachedProperty
 };
 
 
-//-----------------------------------------------------------------------------
-// CCachedGenerator class.
-//-----------------------------------------------------------------------------
-class CCachedGenerator
-{
-public:
-	CCachedGenerator(object generator);
-
-	object get_generator();
-
-	object __iter__();
-
-private:
-	object m_generator;
-	list m_generated_values;
-};
-
-
 #endif // _CORE_CACHE_H

src/core/modules/core/core_cache_wrap.cpp

@@ -36,7 +36,6 @@
 // Forward declarations.
 //-----------------------------------------------------------------------------
 static void export_cached_property(scope);
-static void export_cached_generator(scope);
 
 
 //-----------------------------------------------------------------------------
@@ -45,7 +44,6 @@ static void export_cached_generator(scope);
 DECLARE_SP_SUBMODULE(_core, _cache)
 {
 	export_cached_property(_cache);
-	export_cached_generator(_cache);
 }
 
 
@@ -237,7 +235,7 @@ void export_cached_property(scope _cache)
 
 	CachedProperty.def(
 		"__call__",
-		&CCachedProperty::set_getter,
+		&CCachedProperty::__call__,
 		"Decorator used to register the getter function for this property.\n"
 		"\n"
 		":param function fget:\n"
@@ -267,7 +265,7 @@ void export_cached_property(scope _cache)
 		":param str item:\n"
 		"	The name of the keyword.\n"
 		":param object value:\n"
-		"	The value to assigne to the given keyword."
+		"	The value to assign to the given keyword."
 	);
 
 	CachedProperty.def(
@@ -281,37 +279,3 @@ void export_cached_property(scope _cache)
 
 	scope().attr("cached_property") = scope().attr("CachedProperty");
 }
-
-
-//-----------------------------------------------------------------------------
-// Exports CCachedGenerator.
-//-----------------------------------------------------------------------------
-void export_cached_generator(scope _cache)
-{
-	class_<CCachedGenerator, CCachedGenerator *> CachedGenerator("CachedGenerator",
-		init<object>(
-			(
-				arg("generator")
-			),
-			"Represents a cached generator.\n"
-			"If a :class:`core.cache.CachedProperty` returns a generator, it"
-			" is being cached as an instance of this class. Then, when"
-			" this instance is iterated over for the first time, the original"
-			" generator is processed and the generated values are cached.\n"
-			"\n"
-			":param generator generator:\n"
-			"	The wrapped generator instance.\n"
-			"\n"
-			":raises TypeError:\n"
-			"	If the given generator is invalid.\n"
-			":raises ValueError:\n"
-			"	If the given generator is exhausted."
-		)
-	);
-
-	CachedGenerator.def(
-		"__iter__",
-		&CCachedGenerator::__iter__,
-		"Returns an iterator iterating over the generated values of the wrapped generator."
-	);
-}

src/core/sp_python.cpp

@@ -175,6 +175,7 @@ bool CPythonManager::Initialize( void )
 	if (!modulsp_init())
 	{
 		Msg(MSG_PREFIX "Failed to initialize internal modules.\n");
+		PrintCurrentException();
 		return false;
 	}
 
@@ -230,26 +231,7 @@ bool CPythonManager::Initialize( void )
 
 		// Don't use PyErr_Print() here because our sys.excepthook (might) has not been installed
 		// yet so let's just format and output to the console ourself.
-		if (PyErr_Occurred())
-		{
-			PyObject *pType;
-			PyObject *pValue;
-			PyObject *pTraceback;
-			PyErr_Fetch(&pType, &pValue, &pTraceback);
-			PyErr_NormalizeException(&pType, &pValue, &pTraceback);
-
-			handle<> hType(pType);
-			handle<> hValue(allow_null(pValue));
-			handle<> hTraceback(allow_null(pTraceback));
-
-			object format_exception = import("traceback").attr("format_exception");
-			const char* pMsg = extract<const char *>(str("\n").join(format_exception(hType, hValue, hTraceback)));
-
-			// Send the message in chunks, because it can get quite big.
-			ChunkedMsg(pMsg);
-
-			PyErr_Clear();
-		}
+		PrintCurrentException();
 
 		return false;
 	}
@@ -268,9 +250,8 @@ bool CPythonManager::Shutdown( void )
 		python::import("__init__").attr("unload")();
 	}
 	catch( ... ) {
-		PyErr_Print();
-		PyErr_Clear();
 		Msg(MSG_PREFIX "Failed to unload the main module.\n");
+		PrintCurrentException();
 		return false;
 	}
 	return true;