Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Refcount mess in mvAddCallback() and mvRunCallback() #2282

Open
wants to merge 29 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
4bca26f
Dev branch
bzczb Feb 10, 2024
f700e2d
Preparing drag/drop callbacks... haven't compiled anything yet.
bzczb Feb 11, 2024
e471cac
Still setting up callbacks.
bzczb Feb 11, 2024
6ae6f19
Remove decrementAppData parameter.
bzczb Feb 11, 2024
e6d5102
Compiles and runs.
bzczb Feb 11, 2024
5d94ed5
mvCallbackJob also move-only.
bzczb Feb 11, 2024
c1f823f
mvCallbackJob...
bzczb Feb 11, 2024
3f6651b
mvCallbackJob management.
bzczb Feb 11, 2024
cdf4a52
Fix msLoadingIndicatorCustom.cpp for an updated VC++ compiler.
bzczb Feb 11, 2024
e1073a2
appData refcounts should be good now.
bzczb Feb 11, 2024
0428cec
Whoops. Fix the crash.
bzczb Feb 11, 2024
8efa8f7
More typos.
bzczb Feb 11, 2024
a5e6909
Make sure manual callbacks get None instead of nullptr.
bzczb Feb 11, 2024
5dc323f
Rename mvCallbackSlot -> mvCallbackPoint.
bzczb Feb 11, 2024
6a3f4c9
More bugfixes.
bzczb Feb 11, 2024
5f41ac8
Comments in mvCallbackPoint::set_from_python().
bzczb Feb 11, 2024
3fb0e0a
Drop callback for Linux and MacOS.
bzczb Feb 11, 2024
3867831
Remove drag-and-drop stuff for now, for a pure i2036 fix.
bzczb Feb 12, 2024
360432b
Remove personal scripts and settings.json for PR.
bzczb Feb 12, 2024
af9460d
Remove personal setup.py changes.
bzczb Feb 12, 2024
a25e6fe
Clean up the mvSubmitCallback([](mvAddCallback(...))).
bzczb Feb 12, 2024
90e084d
Rename null_to_none() -> nullToNone().
bzczb Feb 12, 2024
f47fa30
Play better with GIL, keeping most of the complexity in mvCallbackJob.
bzczb Feb 13, 2024
cb0a873
mvDragPayload now uses UUID to lookup, instead of copying the whole o…
bzczb Feb 13, 2024
4f53b4c
Remove GIL locking from mvPyObjectStrict and mvAppItem.
bzczb Feb 13, 2024
fb6cd92
Don't leak callbacks!!
bzczb Feb 13, 2024
90141ec
Fix a few more leaks relating to PyDict_SetItemString().
bzczb Feb 13, 2024
119dda3
Missing include in mvPyUtils.h broke Linux build.
bzczb Mar 23, 2024
a9346c1
Fix some use-after-free in item handlers.
bzczb Mar 29, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -11,4 +11,5 @@ __pycache__/
lib_location.txt
version_number.txt
dearpygui.egg-info/
build/
build/
venv/
4 changes: 4 additions & 0 deletions dearpygui/_dearpygui.pyi

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

10 changes: 10 additions & 0 deletions dearpygui/_dearpygui_RTD.py

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

22 changes: 11 additions & 11 deletions dearpygui/dearpygui.py

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

169 changes: 53 additions & 116 deletions src/dearpygui_commands.h
Original file line number Diff line number Diff line change
Expand Up @@ -1873,15 +1873,10 @@ set_frame_callback(PyObject* self, PyObject* args, PyObject* kwargs)
if (frame > GContext->callbackRegistry->highestFrame)
GContext->callbackRegistry->highestFrame = frame;

// TODO: check previous entry and deprecate if existing
Py_XINCREF(callback);

if(user_data)
Py_XINCREF(user_data);
mvSubmitCallback([=]()
{
GContext->callbackRegistry->frameCallbacks[frame] = callback;
GContext->callbackRegistry->frameCallbacksUserData[frame] = user_data;
GContext->callbackRegistry->frameCallbacks[frame] = std::make_shared<mvPyObjectStrict>(callback);
GContext->callbackRegistry->frameCallbacksUserData[frame] = std::make_shared<mvPyObjectStrict>(user_data);
});

return GetPyNone();
Expand All @@ -1890,47 +1885,13 @@ set_frame_callback(PyObject* self, PyObject* args, PyObject* kwargs)
static PyObject*
set_exit_callback(PyObject* self, PyObject* args, PyObject* kwargs)
{
PyObject* callback;
PyObject* user_data = nullptr;

if (!Parse((GetParsers())["set_exit_callback"], args, kwargs, __FUNCTION__, &callback,
&user_data))
return GetPyNone();

Py_XINCREF(callback);
if(user_data)
Py_XINCREF(user_data);
mvSubmitCallback([=]()
{
GContext->callbackRegistry->onCloseCallback = SanitizeCallback(callback);
GContext->callbackRegistry->onCloseCallbackUserData = user_data;
});
return GetPyNone();
return GContext->callbackRegistry->exitCallbackPoint.set_from_python(self, args, kwargs);
}

static PyObject*
set_viewport_resize_callback(PyObject* self, PyObject* args, PyObject* kwargs)
{
PyObject* callback = nullptr;
PyObject* user_data = nullptr;

if (!Parse((GetParsers())["set_viewport_resize_callback"], args, kwargs, __FUNCTION__,
&callback, &user_data))
return GetPyNone();

if (callback)
Py_XINCREF(callback);

if (user_data)
Py_XINCREF(user_data);

mvSubmitCallback([=]()
{
GContext->callbackRegistry->resizeCallback = SanitizeCallback(callback);
GContext->callbackRegistry->resizeCallbackUserData = user_data;
});

return GetPyNone();
return GContext->callbackRegistry->viewportResizeCallbackPoint.set_from_python(self, args, kwargs);
}

static PyObject*
Expand Down Expand Up @@ -2375,13 +2336,16 @@ output_frame_buffer(PyObject* self, PyObject* args, PyObject* kwargs)

if (filepathLength == 0 && callback) // not specified, return array instead
{
//Py_XINCREF(callback);
auto callback_ptr = std::make_shared<mvPyObjectStrict>(callback);

PyObject* newbuffer = nullptr;
PymvBuffer* newbufferview = PyObject_New(PymvBuffer, &PymvBufferType);
newbuffer = PyObject_Init((PyObject*)newbufferview, &PymvBufferType);
mvSubmitTask([newbuffer, callback, newbufferview]() {
auto appdata_ptr = std::make_shared<mvPyObjectStrict>(newbuffer, false);

mvSubmitTask([appdata_ptr, callback_ptr, newbufferview]() {
OutputFrameBufferArray(newbufferview);
mvAddCallback(callback, 0, newbuffer, nullptr, false);
mvAddCallbackJob({callback_ptr, 0, appdata_ptr, nullptr});
});

return GetPyNone();
Expand Down Expand Up @@ -2511,19 +2475,22 @@ destroy_context(PyObject* self, PyObject* args, PyObject* kwargs)
// true in order to run DPG commands for the
// exit callback.
GContext->started = true;
mvSubmitCallback([=]() {
mvRunCallback(GContext->callbackRegistry->onCloseCallback, 0, nullptr, GContext->callbackRegistry->onCloseCallbackUserData);
auto future = mvSubmitCallback([=]() {
GContext->callbackRegistry->exitCallbackPoint.run_blocking();
GContext->started = false; // return to false after
});

future.get();

ImNodes::DestroyContext();
ImPlot::DestroyContext();
ImGui::DestroyContext();

mvToolManager::Reset();
ClearItemRegistry(*GContext->itemRegistry);


future = mvSubmitCallback([=]() {
mvToolManager::Reset();
ClearItemRegistry(*GContext->itemRegistry);
});
future.get();

//#define X(el) el::s_class_theme_component = nullptr; el::s_class_theme_disabled_component = nullptr;
#define X(el) DearPyGui::GetClassThemeComponent(mvAppItemType::el) = nullptr; DearPyGui::GetDisabledClassThemeComponent(mvAppItemType::el) = nullptr;
Expand All @@ -2535,13 +2502,16 @@ destroy_context(PyObject* self, PyObject* args, PyObject* kwargs)
});
if (GContext->future.valid())
GContext->future.get();
if (GContext->viewport)
delete GContext->viewport;

delete GContext->itemRegistry;
delete GContext->callbackRegistry;
delete GContext;
GContext = nullptr;
{
mvGlobalIntepreterLock gil;
if (GContext->viewport)
delete GContext->viewport;
delete GContext->itemRegistry;
delete GContext->callbackRegistry;
delete GContext;
GContext = nullptr;
}
}
Py_END_ALLOW_THREADS;

Expand Down Expand Up @@ -3603,37 +3573,33 @@ get_item_configuration(PyObject* self, PyObject* args, PyObject* kwargs)
PyDict_SetItemString(pdict, "height", py_height);
PyDict_SetItemString(pdict, "indent", py_indent);

if (appitem->config.callback)
{
Py_XINCREF(appitem->config.callback);
PyDict_SetItemString(pdict, "callback", appitem->config.callback);
if (appitem->config.callback) {
PyDict_SetItemString(pdict, "callback", *appitem->config.callback);
}
else {
PyDict_SetItemString(pdict, "callback", Py_None);
}
else
PyDict_SetItemString(pdict, "callback", GetPyNone());

if (appitem->config.dropCallback)
{
Py_XINCREF(appitem->config.dropCallback);
PyDict_SetItemString(pdict, "drop_callback", appitem->config.dropCallback);
if (appitem->config.dropCallback) {
PyDict_SetItemString(pdict, "drop_callback", *appitem->config.dropCallback);
}
else {
PyDict_SetItemString(pdict, "drop_callback", Py_None);
}
else
PyDict_SetItemString(pdict, "drop_callback", GetPyNone());

if (appitem->config.dragCallback)
{
Py_XINCREF(appitem->config.dragCallback);
PyDict_SetItemString(pdict, "drag_callback", appitem->config.dragCallback);
if (appitem->config.dragCallback) {
PyDict_SetItemString(pdict, "drag_callback", *appitem->config.dragCallback);
}
else {
PyDict_SetItemString(pdict, "drag_callback", Py_None);
}
else
PyDict_SetItemString(pdict, "drag_callback", GetPyNone());

if (appitem->config.user_data)
{
Py_XINCREF(appitem->config.user_data);
PyDict_SetItemString(pdict, "user_data", appitem->config.user_data);
if (appitem->config.user_data) {
PyDict_SetItemString(pdict, "user_data", *appitem->config.user_data);
}
else {
PyDict_SetItemString(pdict, "user_data", Py_None);
}
else
PyDict_SetItemString(pdict, "user_data", GetPyNone());

appitem->getSpecificConfiguration(pdict);
}
Expand Down Expand Up @@ -4061,21 +4027,12 @@ capture_next_item(PyObject* self, PyObject* args, PyObject* kwargs)

std::lock_guard<std::recursive_mutex> lk(GContext->mutex);

if (GContext->itemRegistry->captureCallback)
Py_XDECREF(GContext->itemRegistry->captureCallback);

if (GContext->itemRegistry->captureCallbackUserData)
Py_XDECREF(GContext->itemRegistry->captureCallbackUserData);

Py_XINCREF(callable);
if(user_data)
Py_XINCREF(user_data);
if (callable == Py_None)
GContext->itemRegistry->captureCallback = nullptr;
else
GContext->itemRegistry->captureCallback = callable;
GContext->itemRegistry->captureCallback = std::make_shared<mvPyObjectStrict>(callable);

GContext->itemRegistry->captureCallbackUserData = user_data;
GContext->itemRegistry->captureCallbackUserData = std::make_shared<mvPyObjectStrict>(user_data);

return GetPyNone();
}
Expand All @@ -4087,30 +4044,10 @@ get_callback_queue(PyObject* self, PyObject* args, PyObject* kwargs)
return GetPyNone();

PyObject* pArgs = PyTuple_New(GContext->callbackRegistry->jobs.size());
for (int i = 0; i < GContext->callbackRegistry->jobs.size(); i++)
for (size_t i = 0; i < GContext->callbackRegistry->jobs.size(); i++)
{
PyObject* job = PyTuple_New(4);
if (GContext->callbackRegistry->jobs[i].callback)
PyTuple_SetItem(job, 0, GContext->callbackRegistry->jobs[i].callback);
else
PyTuple_SetItem(job, 0, GetPyNone());

if(GContext->callbackRegistry->jobs[i].sender == 0)
PyTuple_SetItem(job, 1, ToPyString(GContext->callbackRegistry->jobs[i].sender_str));
else
PyTuple_SetItem(job, 1, ToPyUUID(GContext->callbackRegistry->jobs[i].sender));

if (GContext->callbackRegistry->jobs[i].app_data)
PyTuple_SetItem(job, 2, GContext->callbackRegistry->jobs[i].app_data); // steals data, so don't deref
else
PyTuple_SetItem(job, 2, GetPyNone());

if (GContext->callbackRegistry->jobs[i].user_data)
PyTuple_SetItem(job, 3, GContext->callbackRegistry->jobs[i].user_data); // steals data, so don't deref
else
PyTuple_SetItem(job, 3, GetPyNone());

PyTuple_SetItem(pArgs, i, job);
PyObject* job_py = mvCallbackJob::to_python_tuple(std::move(GContext->callbackRegistry->jobs[i]));
PyTuple_SetItem(pArgs, i, job_py);
}

GContext->callbackRegistry->jobs.clear();
Expand Down
2 changes: 2 additions & 0 deletions src/dearpygui_parsers.h
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@
#include "mvViewport.h"
#include <stb_image.h>
#include "mvProfiler.h"
#include <vector>
#include <utility>

static void
InsertParser_Block0(std::map<std::string, mvPythonParser>& parsers)
Expand Down
Loading