Skip to content

Commit 7433e5b

Browse files
author
Roberto De Ioris
committed
refactored memory management to use a gc manager
1 parent 0ae629d commit 7433e5b

9 files changed

Lines changed: 99 additions & 62 deletions

File tree

‎Source/UnrealEnginePython/Private/UEPyModule.cpp‎

Lines changed: 42 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,11 @@
2626

2727
DEFINE_LOG_CATEGORY(LogPython);
2828

29+
30+
31+
32+
33+
2934
PyDoc_STRVAR(unreal_engine_py_doc, "Unreal Engine Python module.");
3035

3136
static PyModuleDef unreal_engine_module = {
@@ -36,8 +41,12 @@ static PyModuleDef unreal_engine_module = {
3641
NULL,
3742
};
3843

44+
UPythonGCManager *PythonGCManager;
3945

4046
static PyObject *init_unreal_engine(void) {
47+
48+
49+
4150
return PyModule_Create(&unreal_engine_module);
4251
}
4352

@@ -254,28 +263,15 @@ static PyMethodDef ue_PyUObject_methods[] = {
254263
};
255264

256265

257-
258-
static void ue_pyobject_clear(ue_PyUObject *self) {
259-
Py_ssize_t len = PyList_Size(self->python_delegates_gc);
260-
for (Py_ssize_t i = 0; i < len; i++) {
261-
PyObject *item = PyList_GetItem(self->python_delegates_gc, i);
262-
UObject *py_delegate = (UObject *)PyLong_AsLongLong(item);
263-
// avoid making mess on engine shutdown
264-
if (!py_delegate || !py_delegate->IsValidLowLevel() || py_delegate->IsPendingKillOrUnreachable())
265-
continue;
266-
if (py_delegate->IsRooted())
267-
py_delegate->RemoveFromRoot();
268-
py_delegate->ConditionalBeginDestroy();
269-
}
270-
#if UEPY_MEMORY_DEBUG
271-
UE_LOG(LogPython, Warning, TEXT("destroying pyobject with %d over %p"), self->python_delegates_gc->ob_refcnt, self->ue_property);
272-
#endif
273-
Py_DECREF(self->python_delegates_gc);
274-
}
275-
276266
// destructor
277267
static void ue_pyobject_dealloc(ue_PyUObject *self) {
278-
ue_pyobject_clear(self);
268+
#if UEPY_MEMORY_DEBUG
269+
UE_LOG(LogPython, Warning, TEXT("Destroying ue_PyUObject %p mapped to UObject %p"), self, self->ue_object);
270+
#endif
271+
// this could happen during engine shutdown, so have mercy
272+
if (PythonGCManager && PythonGCManager->IsValidLowLevel()) {
273+
PythonGCManager->python_properties_gc.Remove(self->ue_property);
274+
}
279275
Py_TYPE(self)->tp_free((PyObject *)self);
280276
}
281277

@@ -371,6 +367,11 @@ void unreal_engine_init_py_module() {
371367
// utility functions
372368

373369
ue_PyUObject *ue_get_python_wrapper(UObject *ue_obj) {
370+
if (!PythonGCManager) {
371+
PythonGCManager = NewObject<UPythonGCManager>();
372+
PythonGCManager->AddToRoot();
373+
UE_LOG(LogPython, Log, TEXT("Initialized Python GC Manager"));
374+
}
374375
if (!ue_obj || !ue_obj->IsValidLowLevel() || ue_obj->IsPendingKillOrUnreachable())
375376
return nullptr;
376377
UPyObject *ue_py_object = FindObject<UPyObject>(ue_obj, TEXT("__PyObject"), true);
@@ -385,16 +386,21 @@ ue_PyUObject *ue_get_python_wrapper(UObject *ue_obj) {
385386
return nullptr;
386387
}
387388
((ue_PyUObject *)ue_py_object->py_object)->ue_object = ue_obj;
388-
((ue_PyUObject *)ue_py_object->py_object)->python_delegates_gc = PyList_New(0);
389389
((ue_PyUObject *)ue_py_object->py_object)->ue_property = ue_py_object;
390+
391+
if (PythonGCManager && PythonGCManager->IsValidLowLevel()) {
392+
PythonGCManager->python_properties_gc.Add(ue_py_object);
393+
}
394+
else {
395+
UE_LOG(LogPython, Fatal, TEXT("[BUG] PYTHON GC MANAGER BROKEN DURING ADD !"));
396+
}
397+
390398
#if UEPY_MEMORY_DEBUG
391-
UE_LOG(LogPython, Warning, TEXT("----------------- CREATED %p"), ue_py_object);
399+
UE_LOG(LogPython, Warning, TEXT("CREATED UPyObject at %p for %p"), ue_py_object, ue_obj);
392400
#endif
393401
//Py_INCREF(ue_py_object->py_object);
394402
}
395-
#if UEPY_MEMORY_DEBUG
396-
UE_LOG(LogPython, Warning, TEXT("REFCNT for %s = %d"), *ue_obj->GetName(), ue_py_object->py_object->ob_base.ob_refcnt);
397-
#endif
403+
398404
return ue_py_object->py_object;
399405
}
400406

@@ -557,13 +563,10 @@ PyObject *ue_py_convert_property(UProperty *prop, uint8 *buffer) {
557563

558564
// map a UStruct to a dictionary (if possible)
559565
if (auto casted_prop = Cast<UStructProperty>(prop)) {
560-
UE_LOG(LogPython, Error, TEXT("UStruct type not supported for %s"), *prop->GetName());
561566
if (auto casted_struct = Cast<UScriptStruct>(casted_prop->Struct)) {
562-
UE_LOG(LogPython, Error, TEXT("UStruct scripted !!!"));
563567
PyObject *py_struct_dict = PyDict_New();
564568
TFieldIterator<UProperty> SArgs(casted_struct);
565569
for (; SArgs; ++SArgs) {
566-
UE_LOG(LogPython, Error, TEXT("UStruct NAME %s"), *SArgs->GetName());
567570
PyObject *struct_value = ue_py_convert_property(*SArgs, casted_prop->ContainerPtrToValuePtr<uint8>(buffer));
568571
if (!struct_value) {
569572
Py_DECREF(py_struct_dict);
@@ -709,12 +712,12 @@ PyObject *ue_bind_pyevent(ue_PyUObject *u_obj, FString event_name, PyObject *py_
709712

710713
FScriptDelegate script_delegate;
711714
UPythonDelegate *py_delegate = NewObject<UPythonDelegate>();
712-
//py_delegate->SetPyCallable(py_callable);
715+
py_delegate->SetPyCallable(py_callable);
713716
py_delegate->SetSignature(casted_prop->SignatureFunction);
717+
// avoid delegates to be destroyed by the GC
714718
py_delegate->AddToRoot();
715-
// collect pointer to allow removal
716-
PyList_Append(u_obj->python_delegates_gc, PyLong_FromLongLong((long long)py_delegate));
717-
719+
u_obj->ue_property->python_delegates_gc.Add(py_delegate);
720+
718721
// fake UFUNCTION for bypassing checks
719722
script_delegate.BindUFunction(py_delegate, FName("PyFakeCallable"));
720723

@@ -735,10 +738,16 @@ PyObject *ue_bind_pyevent(ue_PyUObject *u_obj, FString event_name, PyObject *py_
735738
}
736739

737740
UPyObject::~UPyObject() {
741+
#if UEPY_MEMORY_DEBUG
742+
UE_LOG(LogPython, Error, TEXT("Destroying PyProperty %p"), this);
743+
#endif
744+
for (UPythonDelegate *py_delegate :this->python_delegates_gc) {
745+
py_delegate->RemoveFromRoot();
746+
}
738747
#if UEPY_MEMORY_DEBUG
739748
if (py_object && py_object->ob_base.ob_refcnt != 1) {
740749
UE_LOG(LogPython, Error, TEXT("INCONSISTENT DESTROY %p with refcnt %d"), this, py_object->ob_base.ob_refcnt);
741750
}
742751
#endif
743752
Py_XDECREF(py_object);
744-
}
753+
}

‎Source/UnrealEnginePython/Private/UEPyModule.h‎

Lines changed: 18 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,18 @@
11
#pragma once
22

33
#include "UnrealEnginePython.h"
4+
#include "PythonDelegate.h"
45

56
#include "UEPyModule.generated.h"
67

7-
8+
// fwd declaration
9+
class UPyObject;
810

911
typedef struct {
1012
PyObject_HEAD
11-
/* Type-specific fields go here. */
12-
UObject *ue_object;
13-
UObject *ue_property;
14-
PyObject *python_delegates_gc;
13+
/* Type-specific fields go here. */
14+
UObject *ue_object;
15+
UPyObject *ue_property;
1516
} ue_PyUObject;
1617

1718
UCLASS()
@@ -21,6 +22,17 @@ class UPyObject : public UObject
2122
public:
2223
ue_PyUObject *py_object;
2324
~UPyObject();
25+
TArray<UPythonDelegate*> python_delegates_gc;
26+
};
27+
28+
29+
UCLASS()
30+
class UPythonGCManager : public UObject
31+
{
32+
GENERATED_BODY()
33+
public:
34+
UPROPERTY()
35+
TArray<UPyObject *> python_properties_gc;
2436
};
2537

2638
void unreal_engine_py_log_error();
@@ -32,4 +44,4 @@ bool ue_py_convert_pyobject(PyObject *, UProperty *, uint8 *);
3244
ue_PyUObject *ue_is_pyuobject(PyObject *);
3345

3446
void ue_autobind_events_for_pyclass(ue_PyUObject *, PyObject *);
35-
PyObject *ue_bind_pyevent(ue_PyUObject *, FString, PyObject *, bool);
47+
PyObject *ue_bind_pyevent(ue_PyUObject *, FString, PyObject *, bool);

‎Source/UnrealEnginePython/Public/PyActor.cpp‎

Lines changed: 8 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
#include "UnrealEnginePythonPrivatePCH.h"
22
#include "PyActor.h"
33

4-
4+
#include "PythonDelegate.h"
55

66
APyActor::APyActor()
77
{
@@ -57,29 +57,24 @@ void APyActor::BeginPlay()
5757
return;
5858
}
5959

60-
UE_LOG(LogPython, Warning, TEXT("PyActor refcnt after %s initialization %d"), *this->GetName(), py_actor_instance->ob_refcnt);
61-
62-
ue_PyUObject *py_obj = ue_get_python_wrapper(this);
60+
py_uobject = ue_get_python_wrapper(this);
6361

64-
if (py_obj) {
65-
PyObject_SetAttrString(py_actor_instance, "uobject", (PyObject *) py_obj);
62+
if (py_uobject) {
63+
PyObject_SetAttrString(py_actor_instance, "uobject", (PyObject *)py_uobject);
6664
}
6765
else {
6866
UE_LOG(LogPython, Error, TEXT("Unable to set 'uobject' field in actor wrapper class"));
67+
unreal_engine_py_log_error();
68+
return;
6969
}
7070

71-
UE_LOG(LogPython, Warning, TEXT("PyActor refcnt after %s initialization[2] %d"), *this->GetName(), py_actor_instance->ob_refcnt);
72-
7371
if (!PyObject_HasAttrString(py_actor_instance, "tick") || PythonTickForceDisabled) {
7472
SetActorTickEnabled(false);
7573
}
7674

77-
UE_LOG(LogPython, Warning, TEXT("PyActor refcnt before %s autobinding %d"), *this->GetName(), py_actor_instance->ob_refcnt);
78-
7975
if (!PythonDisableAutoBinding)
80-
ue_autobind_events_for_pyclass(py_obj, py_actor_instance);
76+
ue_autobind_events_for_pyclass(py_uobject, py_actor_instance);
8177

82-
UE_LOG(LogPython, Warning, TEXT("PyActor refcnt after %s autobinding %d"), *this->GetName(), py_actor_instance->ob_refcnt);
8378

8479
if (!PyObject_HasAttrString(py_actor_instance, "begin_play"))
8580
return;
@@ -203,6 +198,6 @@ APyActor::~APyActor()
203198
Py_XDECREF(py_actor_instance);
204199

205200
#if UEPY_MEMORY_DEBUG
206-
UE_LOG(LogPython, Warning, TEXT("Python AActor %s wrapper XDECREF'ed"), *this->GetName());
201+
UE_LOG(LogPython, Warning, TEXT("Python AActor (mapped to %p) wrapper XDECREF'ed"), py_uobject ? py_uobject->ue_object: nullptr);
207202
#endif
208203
}

‎Source/UnrealEnginePython/Public/PyActor.h‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,5 +46,7 @@ class APyActor : public AActor
4646

4747
private:
4848
PyObject *py_actor_instance;
49+
// mapped uobject, required for debug and advanced reflection
50+
ue_PyUObject *py_uobject;
4951
};
5052

‎Source/UnrealEnginePython/Public/PyPawn.cpp‎

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -56,13 +56,15 @@ void APyPawn::BeginPlay()
5656
return;
5757
}
5858

59-
ue_PyUObject *py_obj = ue_get_python_wrapper(this);
59+
py_uobject = ue_get_python_wrapper(this);
6060

61-
if (py_obj) {
62-
PyObject_SetAttrString(py_pawn_instance, "uobject", (PyObject *) py_obj);
61+
if (py_uobject) {
62+
PyObject_SetAttrString(py_pawn_instance, "uobject", (PyObject *) py_uobject);
6363
}
6464
else {
6565
UE_LOG(LogPython, Error, TEXT("Unable to set 'uobject' field in pawn wrapper class"));
66+
unreal_engine_py_log_error();
67+
return;
6668
}
6769

6870
// disable ticking if not required
@@ -71,7 +73,7 @@ void APyPawn::BeginPlay()
7173
}
7274

7375
if (!PythonDisableAutoBinding)
74-
ue_autobind_events_for_pyclass(py_obj, py_pawn_instance);
76+
ue_autobind_events_for_pyclass(py_uobject, py_pawn_instance);
7577

7678
if (!PyObject_HasAttrString(py_pawn_instance, "begin_play"))
7779
return;
@@ -165,8 +167,13 @@ FString APyPawn::CallPythonPawnMethodString(FString method_name)
165167

166168
APyPawn::~APyPawn()
167169
{
170+
#if UEPY_MEMORY_DEBUG
171+
if (py_pawn_instance && py_pawn_instance->ob_refcnt != 1) {
172+
UE_LOG(LogPython, Error, TEXT("Inconsistent Python APawn wrapper refcnt = %d"), py_pawn_instance->ob_refcnt);
173+
}
174+
#endif
168175
Py_XDECREF(py_pawn_instance);
169176
#if UEPY_MEMORY_DEBUG
170-
UE_LOG(LogPython, Warning, TEXT("Python APawn wrapper XDECREF'ed"));
177+
UE_LOG(LogPython, Warning, TEXT("Python APawn (mapped to %p) wrapper XDECREF'ed"), py_uobject ? py_uobject->ue_object : nullptr);
171178
#endif
172179
}

‎Source/UnrealEnginePython/Public/PyPawn.h‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,5 +46,7 @@ class APyPawn : public APawn
4646

4747
private:
4848
PyObject *py_pawn_instance;
49+
// mapped uobject, required for debug and advanced reflection
50+
ue_PyUObject *py_uobject;
4951
};
5052

‎Source/UnrealEnginePython/Public/PythonComponent.cpp‎

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -60,13 +60,15 @@ void UPythonComponent::BeginPlay()
6060
return;
6161
}
6262

63-
ue_PyUObject *py_obj = ue_get_python_wrapper(this);
63+
py_uobject = ue_get_python_wrapper(this);
6464

65-
if (py_obj) {
66-
PyObject_SetAttrString(py_component_instance, "uobject", (PyObject *)py_obj);
65+
if (py_uobject) {
66+
PyObject_SetAttrString(py_component_instance, "uobject", (PyObject *)py_uobject);
6767
}
6868
else {
6969
UE_LOG(LogPython, Error, TEXT("Unable to set 'uobject' field in component wrapper class"));
70+
unreal_engine_py_log_error();
71+
return;
7072
}
7173

7274
// disable ticking if no tick method is exposed
@@ -75,7 +77,7 @@ void UPythonComponent::BeginPlay()
7577
}
7678

7779
if (!PythonDisableAutoBinding)
78-
ue_autobind_events_for_pyclass(py_obj, py_component_instance);
80+
ue_autobind_events_for_pyclass(py_uobject, py_component_instance);
7981

8082
if (!PyObject_HasAttrString(py_component_instance, "begin_play")) {
8183
return;
@@ -193,8 +195,13 @@ FString UPythonComponent::CallPythonComponentMethodString(FString method_name, F
193195

194196
UPythonComponent::~UPythonComponent()
195197
{
198+
#if UEPY_MEMORY_DEBUG
199+
if (py_component_instance && py_component_instance->ob_refcnt != 1) {
200+
UE_LOG(LogPython, Error, TEXT("Inconsistent Python UActorComponent wrapper refcnt = %d"), py_component_instance->ob_refcnt);
201+
}
202+
#endif
196203
Py_XDECREF(py_component_instance);
197204
#if UEPY_MEMORY_DEBUG
198-
UE_LOG(LogPython, Warning, TEXT("Python UActorComponent wrapper XDECREF'ed"));
205+
UE_LOG(LogPython, Warning, TEXT("Python UActorComponent (mapped to %p) wrapper XDECREF'ed"), py_uobject ? py_uobject->ue_object : nullptr);
199206
#endif
200207
}

‎Source/UnrealEnginePython/Public/PythonComponent.h‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,5 +45,7 @@ class UPythonComponent : public UActorComponent
4545

4646
private:
4747
PyObject *py_component_instance;
48+
// mapped uobject, required for debug and advanced reflection
49+
ue_PyUObject *py_uobject;
4850
};
4951

‎Source/UnrealEnginePython/Public/UnrealEnginePython.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66

77
DECLARE_LOG_CATEGORY_EXTERN(LogPython, Log, All);
88

9+
910
class FUnrealEnginePythonModule : public IModuleInterface
1011
{
1112
public:

0 commit comments

Comments
 (0)