Skip to content

Commit 45a4060

Browse files
committed
Added back CachedProperty.owner as a weak reference.
Reverted EntityMemFuncWrapper back to be bound and store a proxy of the wrapped instance. My tests were not leaking because the hash was matching so the cache was re-used but no matter where we cache the value, it will still keep the instance hostage if it strongly refers it. Left the unbound implementation in place, as it can be useful in other cases. Added unbound parameter to CachedProperty.wrap_descriptor. Updated docstrings and signatures.
1 parent 88bec54 commit 45a4060

File tree

5 files changed

+70
-21
lines changed

5 files changed

+70
-21
lines changed

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

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,10 @@
55
# =============================================================================
66
# >> IMPORTS
77
# =============================================================================
8+
# Python Imports
9+
# WeakRef
10+
from weakref import proxy
11+
812
# Source.Python Imports
913
# Core
1014
from core.cache import CachedProperty
@@ -97,7 +101,7 @@ def __init__(self, wrapped_self, wrapper):
97101
func = wrapped_self.__getattr__(wrapper.__name__)
98102
super().__init__(func._manager, func._type_name, func, func._this)
99103
self.wrapper = wrapper
100-
self.wrapped_self = wrapped_self
104+
self.wrapped_self = proxy(wrapped_self)
101105

102106
def __call__(self, *args, **kwargs):
103107
return super().__call__(
@@ -120,6 +124,5 @@ def wrap_entity_mem_func(wrapper):
120124

121125
return CachedProperty(
122126
lambda self: EntityMemFuncWrapper(self, wrapper),
123-
doc=wrapper.__doc__,
124-
unbound=True
127+
doc=wrapper.__doc__
125128
)

src/core/modules/core/core_cache.cpp

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -191,10 +191,16 @@ str CCachedProperty::get_name()
191191
return m_name;
192192
}
193193

194+
object CCachedProperty::get_owner()
195+
{
196+
return m_owner();
197+
}
198+
194199

195200
void CCachedProperty::__set_name__(object owner, str name)
196201
{
197202
m_name = name;
203+
m_owner = object(handle<>(PyWeakref_NewRef(owner.ptr(), NULL)));
198204
}
199205

200206
object CCachedProperty::__get__(object self, object instance, object owner=object())
@@ -302,13 +308,14 @@ void CCachedProperty::__setitem__(str item, object value)
302308
}
303309

304310

305-
CCachedProperty *CCachedProperty::wrap_descriptor(object descriptor, object owner=object(), str name=str())
311+
CCachedProperty *CCachedProperty::wrap_descriptor(
312+
object descriptor, object owner=object(), str name=str(), bool unbound=false)
306313
{
307314
CCachedProperty *pProperty = new CCachedProperty(
308-
descriptor.attr("__get__"), descriptor.attr("__set__"), descriptor.attr("__delete__")
315+
descriptor.attr("__get__"), descriptor.attr("__set__"), descriptor.attr("__delete__"),
316+
extract<const char *>(descriptor.attr("__doc__")), unbound
309317
);
310318

311-
pProperty->m_szDocString = extract<const char *>(descriptor.attr("__doc__"));
312319
pProperty->__set_name__(owner, name);
313320

314321
return pProperty;

src/core/modules/core/core_cache.h

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,7 @@ class CCachedProperty
6161
object set_deleter(object fget);
6262

6363
str get_name();
64+
object get_owner();
6465

6566
void __set_name__(object owner, str name);
6667
static object __get__(object self, object instance, object owner);
@@ -70,14 +71,15 @@ class CCachedProperty
7071
object __getitem__(str item);
7172
void __setitem__(str item, object value);
7273

73-
static CCachedProperty *wrap_descriptor(object descriptor, object owner, str name);
74+
static CCachedProperty *wrap_descriptor(object descriptor, object owner, str name, bool unbound);
7475

7576
private:
7677
object m_fget;
7778
object m_fset;
7879
object m_fdel;
7980

8081
str m_name;
82+
object m_owner;
8183

8284
bool m_bUnbound;
8385
dict m_cache;

src/core/modules/core/core_cache_wrap.cpp

Lines changed: 50 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ void export_cached_property(scope _cache)
5656
"CachedProperty",
5757
init<object, object, object, const char *, bool, boost::python::tuple, dict>(
5858
(
59-
arg("fget")=object(), arg("fset")=object(), arg("fdel")=object(), arg("doc")=object(),
59+
arg("self"), arg("fget")=object(), arg("fset")=object(), arg("fdel")=object(), arg("doc")=object(),
6060
arg("unbound")=false, arg("args")=boost::python::tuple(), arg("kwargs")=dict()
6161
),
6262
"Represents a property attribute that is only"
@@ -83,10 +83,10 @@ void export_cached_property(scope _cache)
8383
" Deleter signature: self, *args, **kwargs\n"
8484
":param str doc:\n"
8585
" Documentation string for this property.\n"
86-
":param bool unbound\n"
86+
":param bool unbound:\n"
8787
" Whether the cached objects should be independently maintained rather than bound to"
8888
" the instance they belong to. The cache will be slightly slower to lookup, but this can"
89-
" be required in certain cases to prevent circular references/memory leak.\n"
89+
" be required for instances that do not have a `__dict__` attribute.\n"
9090
":param tuple args:\n"
9191
" Extra arguments passed to the getter, setter and deleter functions.\n"
9292
":param dict kwargs:\n"
@@ -111,7 +111,8 @@ void export_cached_property(scope _cache)
111111
" The function to register as getter function.\n"
112112
"\n"
113113
":rtype:\n"
114-
" function"
114+
" function",
115+
args("self", "fget")
115116
);
116117

117118
CachedProperty.add_property(
@@ -134,7 +135,8 @@ void export_cached_property(scope _cache)
134135
" The function to register as setter function.\n"
135136
"\n"
136137
":rtype:\n"
137-
" function"
138+
" function",
139+
args("self", "fset")
138140
);
139141

140142
CachedProperty.add_property(
@@ -157,7 +159,8 @@ void export_cached_property(scope _cache)
157159
" The function to register as deleter function.\n"
158160
"\n"
159161
":rtype:\n"
160-
" function"
162+
" function",
163+
args("self", "fdel")
161164
);
162165

163166
CachedProperty.add_property(
@@ -190,6 +193,16 @@ void export_cached_property(scope _cache)
190193
" str"
191194
);
192195

196+
CachedProperty.add_property(
197+
"owner",
198+
&CCachedProperty::get_owner,
199+
"The owner class this property attribute was bound to.\n"
200+
"\n"
201+
":rtype:\n"
202+
" type"
203+
);
204+
205+
193206
CachedProperty.def_readwrite(
194207
"args",
195208
&CCachedProperty::m_args,
@@ -212,30 +225,51 @@ void export_cached_property(scope _cache)
212225
"__set_name__",
213226
&CCachedProperty::__set_name__,
214227
"Called when this property is being bound to a class.\n"
228+
"\n"
229+
":param class owner:\n"
230+
" The class this property is being bound to.\n"
231+
":param str name:\n"
232+
" The name this property is being bound as.",
233+
args("self", "owner", "name")
215234
);
216235

217236
CachedProperty.def(
218237
"__get__",
219238
&CCachedProperty::__get__,
220239
"Retrieves the value of this property.\n"
221240
"\n"
241+
":param object instance:\n"
242+
" The instance for which this property is retrieved.\n"
243+
":param class owner:\n"
244+
" The class for which this property is retrieved.\n"
245+
"\n"
222246
":rtype:\n"
223-
" object"
247+
" object",
248+
("self", "instance", arg("owner")=object())
224249
);
225250

226251
CachedProperty.def(
227252
"__set__",
228253
&CCachedProperty::__set__,
229254
"Assigns the value of this property.\n"
230255
"\n"
256+
":param object instance:\n"
257+
" The instance this property is being assigned to.\n"
258+
":param object value:\n"
259+
" The value assigned to this property.\n"
231260
":rtype:\n"
232-
" object"
261+
" object",
262+
args("self", "instance", "value")
233263
);
234264

235265
CachedProperty.def(
236266
"__delete__",
237267
&CCachedProperty::__delete__,
238-
"Deletes this property and invalidates its cached value."
268+
"Deletes this property and invalidates its cached value.\n"
269+
"\n"
270+
":param object instance:\n"
271+
" The instance for which this property if being deleted.",
272+
args("self", "instance")
239273
);
240274

241275
CachedProperty.def(
@@ -247,7 +281,8 @@ void export_cached_property(scope _cache)
247281
" The function to register as getter function.\n"
248282
"\n"
249283
":rtype:\n"
250-
" function"
284+
" function",
285+
args("self", "fget")
251286
);
252287

253288
CachedProperty.def(
@@ -259,7 +294,8 @@ void export_cached_property(scope _cache)
259294
" The name of the keyword.\n"
260295
"\n"
261296
":rtype:"
262-
" object"
297+
" object",
298+
args("self", "item")
263299
);
264300

265301
CachedProperty.def(
@@ -270,7 +306,8 @@ void export_cached_property(scope _cache)
270306
":param str item:\n"
271307
" The name of the keyword.\n"
272308
":param object value:\n"
273-
" The value to assign to the given keyword."
309+
" The value to assign to the given keyword.",
310+
args("self", "item", "value")
274311
);
275312

276313
CachedProperty.def(
@@ -292,7 +329,7 @@ void export_cached_property(scope _cache)
292329
" If the given descriptor doesn't have the required methods.\n"
293330
":raises TypeError:\n"
294331
" If the getter, setter or deleter are not callable.",
295-
("descriptor", arg("owner")=object(), arg("name")=str())
332+
("descriptor", arg("owner")=object(), arg("name")=str(), arg("unbound")=false)
296333
)
297334
.staticmethod("wrap_descriptor");
298335

src/core/utilities/wrap_macros.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -164,7 +164,7 @@ template<typename T>
164164
T cached_property(T cls, const char *szName)
165165
{
166166
cls.attr(szName) = transfer_ownership_to_python(
167-
CCachedProperty::wrap_descriptor(cls.attr(szName), cls, szName)
167+
CCachedProperty::wrap_descriptor(cls.attr(szName), cls, szName, false)
168168
);
169169
return cls;
170170
};

0 commit comments

Comments
 (0)