From: Wojtek Porczyk <woju@xxxxxxxxxxxxxxxxxxxxxx>
The documentation says:
> If the opaque user data requires free'ing when the handle is
> unregistered, then a 2nd callback can be supplied for this purpose.
> This callback needs to be invoked from a clean stack. If 'ff'
> callbacks are invoked directly from the virEventRemoveHandleFunc they
> will likely deadlock in libvirt.
And they did deadlock. In removeTimeout too. Now we supply a custom
function to pick it from the opaque blob and fire.
Signed-off-by: Wojtek Porczyk <woju@xxxxxxxxxxxxxxxxxxxxxx>
Signed-off-by: Daniel P. Berrange <berrange@xxxxxxxxxx>
---
libvirt-override.c | 68 ++++++++++++++++++++++++++++++-----------------------
libvirt-override.py | 23 ++++++++++++++++++
sanitytest.py | 3 ++-
3 files changed, 63 insertions(+), 31 deletions(-)
diff --git a/libvirt-override.c b/libvirt-override.c
index 93c7ef0..a762941 100644
--- a/libvirt-override.c
+++ b/libvirt-override.c
@@ -5223,6 +5223,9 @@ libvirt_virEventAddHandleFunc(int fd,
VIR_PY_TUPLE_SET_GOTO(pyobj_args, 3, cb_args, cleanup);
+ /* If changing contents of the opaque object, please also change
+ * virEventInvokeFreeCallback() in libvirt-override.py
+ */
VIR_PY_TUPLE_SET_GOTO(cb_args, 0, libvirt_virEventHandleCallbackWrap(cb), cleanup);
VIR_PY_TUPLE_SET_GOTO(cb_args, 1, libvirt_virVoidPtrWrap(opaque), cleanup);
VIR_PY_TUPLE_SET_GOTO(cb_args, 2, libvirt_virFreeCallbackWrap(ff), cleanup);
@@ -5279,10 +5282,7 @@ libvirt_virEventRemoveHandleFunc(int watch)
{
PyObject *result = NULL;
PyObject *pyobj_args;
- PyObject *opaque;
- PyObject *ff;
int retval = -1;
- virFreeCallback cff;
LIBVIRT_ENSURE_THREAD_STATE;
@@ -5292,20 +5292,11 @@ libvirt_virEventRemoveHandleFunc(int watch)
VIR_PY_TUPLE_SET_GOTO(pyobj_args, 0, libvirt_intWrap(watch), cleanup);
result = PyEval_CallObject(removeHandleObj, pyobj_args);
- if (!result) {
+ if (result) {
+ retval = 0;
+ } else {
PyErr_Print();
PyErr_Clear();
- } else if (!PyTuple_Check(result) || PyTuple_Size(result) != 3) {
- DEBUG("%s: %s must return opaque obj registered with %s"
- "to avoid leaking libvirt memory\n",
- __FUNCTION__, NAME(removeHandle), NAME(addHandle));
- } else {
- opaque = PyTuple_GetItem(result, 1);
- ff = PyTuple_GetItem(result, 2);
- cff = PyvirFreeCallback_Get(ff);
- if (cff)
- (*cff)(PyvirVoidPtr_Get(opaque));
- retval = 0;
}
cleanup:
@@ -5350,6 +5341,9 @@ libvirt_virEventAddTimeoutFunc(int timeout,
VIR_PY_TUPLE_SET_GOTO(pyobj_args, 2, cb_args, cleanup);
+ /* If changing contents of the opaque object, please also change
+ * virEventInvokeFreeCallback() in libvirt-override.py
+ */
VIR_PY_TUPLE_SET_GOTO(cb_args, 0, libvirt_virEventTimeoutCallbackWrap(cb), cleanup);
VIR_PY_TUPLE_SET_GOTO(cb_args, 1, libvirt_virVoidPtrWrap(opaque), cleanup);
VIR_PY_TUPLE_SET_GOTO(cb_args, 2, libvirt_virFreeCallbackWrap(ff), cleanup);
@@ -5403,10 +5397,7 @@ libvirt_virEventRemoveTimeoutFunc(int timer)
{
PyObject *result = NULL;
PyObject *pyobj_args;
- PyObject *opaque;
- PyObject *ff;
int retval = -1;
- virFreeCallback cff;
LIBVIRT_ENSURE_THREAD_STATE;
@@ -5416,20 +5407,11 @@ libvirt_virEventRemoveTimeoutFunc(int timer)
VIR_PY_TUPLE_SET_GOTO(pyobj_args, 0, libvirt_intWrap(timer), cleanup);
result = PyEval_CallObject(removeTimeoutObj, pyobj_args);
- if (!result) {
+ if (result) {
+ retval = 0;
+ } else {
PyErr_Print();
PyErr_Clear();
- } else if (!PyTuple_Check(result) || PyTuple_Size(result) != 3) {
- DEBUG("%s: %s must return opaque obj registered with %s"
- "to avoid leaking libvirt memory\n",
- __FUNCTION__, NAME(removeTimeout), NAME(addTimeout));
- } else {
- opaque = PyTuple_GetItem(result, 1);
- ff = PyTuple_GetItem(result, 2);
- cff = PyvirFreeCallback_Get(ff);
- if (cff)
- (*cff)(PyvirVoidPtr_Get(opaque));
- retval = 0;
}
cleanup:
@@ -5558,6 +5540,31 @@ libvirt_virEventInvokeTimeoutCallback(PyObject *self ATTRIBUTE_UNUSED,
return VIR_PY_INT_SUCCESS;
}
+static PyObject *
+libvirt_virEventInvokeFreeCallback(PyObject *self ATTRIBUTE_UNUSED,
+ PyObject *args)
+{
+ PyObject *py_f;
+ PyObject *py_opaque;
+ virFreeCallback cb;
+ void *opaque;
+
+ if (!PyArg_ParseTuple(args, (char *) "OO:virEventInvokeFreeCallback",
+ &py_f, &py_opaque))
+ return NULL;
+
+ cb = (virFreeCallback) PyvirEventHandleCallback_Get(py_f);
+ opaque = (void *) PyvirVoidPtr_Get(py_opaque);
+
+ if (cb) {
+ LIBVIRT_BEGIN_ALLOW_THREADS;
+ cb(opaque);
+ LIBVIRT_END_ALLOW_THREADS;
+ }
+
+ return VIR_PY_INT_SUCCESS;
+}
+
static void
libvirt_virEventHandleCallback(int watch,
int fd,
@@ -9572,6 +9579,7 @@ static PyMethodDef libvirtMethods[] = {
{(char *) "virEventAddTimeout", libvirt_virEventAddTimeout, METH_VARARGS, NULL},
{(char *) "virEventInvokeHandleCallback", libvirt_virEventInvokeHandleCallback, METH_VARARGS, NULL},
{(char *) "virEventInvokeTimeoutCallback", libvirt_virEventInvokeTimeoutCallback, METH_VARARGS, NULL},
+ {(char *) "virEventInvokeFreeCallback", libvirt_virEventInvokeFreeCallback, METH_VARARGS, NULL},
{(char *) "virNodeListDevices", libvirt_virNodeListDevices, METH_VARARGS, NULL},
#if LIBVIR_CHECK_VERSION(0, 10, 2)
{(char *) "virConnectListAllNodeDevices", libvirt_virConnectListAllNodeDevices, METH_VARARGS, NULL},
diff --git a/libvirt-override.py b/libvirt-override.py
index 63f8ecb..b0b24ef 100644
--- a/libvirt-override.py
+++ b/libvirt-override.py
@@ -211,3 +211,26 @@ def virEventAddTimeout(timeout, cb, opaque):
ret = libvirtmod.virEventAddTimeout(timeout, cbData)
if ret == -1: raise libvirtError ('virEventAddTimeout() failed')
return ret
+
+
+#
+# a caller for the ff callbacks for custom event loop implementations
+#
+
+def virEventInvokeFreeCallback(opaque):
+ """
+ Execute callback which frees the opaque buffer
+
+ @opaque: the opaque object passed to addHandle or addTimeout
+
+ WARNING: This function should not be called from any call by libvirt's
+ core. It will most probably cause deadlock in C-level libvirt code.
+ Instead it should be scheduled and called from implementation's stack.
+
+ See https://libvirt.org/html/libvirt-libvirt-event.html#virEventAddHandleFunc
+ for more information.
+
+ This function is not dependent on any event loop implementation.
+ """
+
+ libvirtmod.virEventInvokeFreeCallback(opaque[2], opaque[1])
diff --git a/sanitytest.py b/sanitytest.py
index a140ba2..7183baa 100644
--- a/sanitytest.py
+++ b/sanitytest.py
@@ -349,7 +349,8 @@ for klass in gotfunctions:
continue
for func in sorted(gotfunctions[klass]):
# These are pure python methods with no C APi
- if func in ["connect", "getConnect", "domain", "getDomain"]:
+ if func in ["connect", "getConnect", "domain", "getDomain",
+ "virEventInvokeFreeCallback"]:
continue
key = "%s.%s" % (klass, func)
--
2.9.3
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
