src: make virObjectUnref return void

To prepare for a conversion to GObject, we need virObjectUnref
to have the same API design as g_object_unref, which means it
needs to be void.

A few places do actually care about the return value though,
and in these cases a thread local flag is used to determine
if the dispose method was invoked.

Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
This commit is contained in:
Daniel P. Berrangé 2020-05-15 16:36:00 +01:00
parent fd460ef561
commit 0d1840729f
13 changed files with 81 additions and 22 deletions

View File

@ -295,7 +295,9 @@ virAdmConnectClose(virAdmConnectPtr conn)
virCheckAdmConnectReturn(conn, -1); virCheckAdmConnectReturn(conn, -1);
if (!virObjectUnref(conn)) virAdmConnectWatchDispose();
virObjectUnref(conn);
if (virAdmConnectWasDisposed())
return 0; return 0;
return 1; return 1;
} }

View File

@ -76,6 +76,9 @@ virClassPtr virAdmClientClass;
static void virAdmServerDispose(void *obj); static void virAdmServerDispose(void *obj);
static void virAdmClientDispose(void *obj); static void virAdmClientDispose(void *obj);
static __thread bool connectDisposed;
static __thread bool admConnectDisposed;
static int static int
virDataTypesOnceInit(void) virDataTypesOnceInit(void)
{ {
@ -133,6 +136,27 @@ virGetConnect(void)
return virObjectLockableNew(virConnectClass); return virObjectLockableNew(virConnectClass);
} }
void virConnectWatchDispose(void)
{
connectDisposed = false;
}
bool virConnectWasDisposed(void)
{
return connectDisposed;
}
void virAdmConnectWatchDispose(void)
{
admConnectDisposed = false;
}
bool virAdmConnectWasDisposed(void)
{
return admConnectDisposed;
}
/** /**
* virConnectDispose: * virConnectDispose:
* @obj: the hypervisor connection to release * @obj: the hypervisor connection to release
@ -145,6 +169,7 @@ virConnectDispose(void *obj)
{ {
virConnectPtr conn = obj; virConnectPtr conn = obj;
connectDisposed = true;
if (conn->driver) if (conn->driver)
conn->driver->connectClose(conn); conn->driver->connectClose(conn);
@ -1092,6 +1117,7 @@ virAdmConnectDispose(void *obj)
{ {
virAdmConnectPtr conn = obj; virAdmConnectPtr conn = obj;
admConnectDisposed = true;
if (conn->privateDataFreeFunc) if (conn->privateDataFreeFunc)
conn->privateDataFreeFunc(conn); conn->privateDataFreeFunc(conn);

View File

@ -843,6 +843,12 @@ virAdmClientPtr virAdmGetClient(virAdmServerPtr srv,
unsigned long long timestamp, unsigned long long timestamp,
unsigned int transport); unsigned int transport);
/* Thread local to watch if an ObjectUnref causes a Dispoe */
void virConnectWatchDispose(void);
bool virConnectWasDisposed(void);
void virAdmConnectWatchDispose(void);
bool virAdmConnectWasDisposed(void);
virConnectCloseCallbackDataPtr virNewConnectCloseCallbackData(void); virConnectCloseCallbackDataPtr virNewConnectCloseCallbackData(void);
void virConnectCloseCallbackDataRegister(virConnectCloseCallbackDataPtr close, void virConnectCloseCallbackDataRegister(virConnectCloseCallbackDataPtr close,
virConnectPtr conn, virConnectPtr conn,

View File

@ -148,12 +148,7 @@ netcfStateCleanup(void)
if (!driver) if (!driver)
return -1; return -1;
if (virObjectUnref(driver)) { virObjectUnref(driver);
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("Attempt to close netcf state driver "
"with open connections"));
return -1;
}
driver = NULL; driver = NULL;
return 0; return 0;
} }

View File

@ -1277,7 +1277,9 @@ virConnectClose(virConnectPtr conn)
virCheckConnectReturn(conn, -1); virCheckConnectReturn(conn, -1);
if (!virObjectUnref(conn)) virConnectWatchDispose();
virObjectUnref(conn);
if (virConnectWasDisposed())
return 0; return 0;
return 1; return 1;
} }

View File

@ -6691,8 +6691,10 @@ qemuDomainObjExitMonitorInternal(virQEMUDriverPtr driver,
qemuDomainObjPrivatePtr priv = obj->privateData; qemuDomainObjPrivatePtr priv = obj->privateData;
bool hasRefs; bool hasRefs;
hasRefs = virObjectUnref(priv->mon); qemuMonitorWatchDispose();
virObjectUnref(priv->mon);
hasRefs = !qemuMonitorWasDisposed();
if (hasRefs) if (hasRefs)
virObjectUnlock(priv->mon); virObjectUnlock(priv->mon);

View File

@ -146,6 +146,7 @@ struct _qemuMonitor {
QEMU_CHECK_MONITOR_FULL(mon, goto label) QEMU_CHECK_MONITOR_FULL(mon, goto label)
static virClassPtr qemuMonitorClass; static virClassPtr qemuMonitorClass;
static __thread bool qemuMonitorDisposed;
static void qemuMonitorDispose(void *obj); static void qemuMonitorDispose(void *obj);
static int qemuMonitorOnceInit(void) static int qemuMonitorOnceInit(void)
@ -222,6 +223,7 @@ qemuMonitorDispose(void *obj)
qemuMonitorPtr mon = obj; qemuMonitorPtr mon = obj;
VIR_DEBUG("mon=%p", mon); VIR_DEBUG("mon=%p", mon);
qemuMonitorDisposed = true;
if (mon->cb && mon->cb->destroy) if (mon->cb && mon->cb->destroy)
(mon->cb->destroy)(mon, mon->vm, mon->callbackOpaque); (mon->cb->destroy)(mon, mon->vm, mon->callbackOpaque);
virObjectUnref(mon->vm); virObjectUnref(mon->vm);
@ -799,6 +801,18 @@ qemuMonitorOpen(virDomainObjPtr vm,
} }
void qemuMonitorWatchDispose(void)
{
qemuMonitorDisposed = false;
}
bool qemuMonitorWasDisposed(void)
{
return qemuMonitorDisposed;
}
/** /**
* qemuMonitorRegister: * qemuMonitorRegister:
* @mon: QEMU monitor * @mon: QEMU monitor

View File

@ -387,6 +387,9 @@ qemuMonitorPtr qemuMonitorOpen(virDomainObjPtr vm,
void *opaque) void *opaque)
ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(5); ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(5);
void qemuMonitorWatchDispose(void);
bool qemuMonitorWasDisposed(void);
void qemuMonitorRegister(qemuMonitorPtr mon) void qemuMonitorRegister(qemuMonitorPtr mon)
ATTRIBUTE_NONNULL(1); ATTRIBUTE_NONNULL(1);
void qemuMonitorUnregister(qemuMonitorPtr mon) void qemuMonitorUnregister(qemuMonitorPtr mon)

View File

@ -128,6 +128,7 @@ static testDriverPtr defaultPrivconn;
static virMutex defaultLock = VIR_MUTEX_INITIALIZER; static virMutex defaultLock = VIR_MUTEX_INITIALIZER;
static virClassPtr testDriverClass; static virClassPtr testDriverClass;
static __thread bool testDriverDisposed;
static void testDriverDispose(void *obj); static void testDriverDispose(void *obj);
static int testDriverOnceInit(void) static int testDriverOnceInit(void)
{ {
@ -171,6 +172,8 @@ testDriverDispose(void *obj)
g_free(driver->auths[i].password); g_free(driver->auths[i].password);
} }
g_free(driver->auths); g_free(driver->auths);
testDriverDisposed = true;
} }
typedef struct _testDomainNamespaceDef testDomainNamespaceDef; typedef struct _testDomainNamespaceDef testDomainNamespaceDef;
@ -1446,8 +1449,9 @@ static void
testDriverCloseInternal(testDriverPtr driver) testDriverCloseInternal(testDriverPtr driver)
{ {
virMutexLock(&defaultLock); virMutexLock(&defaultLock);
bool disposed = !virObjectUnref(driver); testDriverDisposed = false;
if (disposed && driver == defaultPrivconn) virObjectUnref(driver);
if (testDriverDisposed && driver == defaultPrivconn)
defaultPrivconn = NULL; defaultPrivconn = NULL;
virMutexUnlock(&defaultLock); virMutexUnlock(&defaultLock);
} }

View File

@ -112,6 +112,7 @@ struct virFDStreamData {
}; };
static virClassPtr virFDStreamDataClass; static virClassPtr virFDStreamDataClass;
static __thread bool virFDStreamDataDisposed;
static void virFDStreamMsgQueueFree(virFDStreamMsgPtr *queue); static void virFDStreamMsgQueueFree(virFDStreamMsgPtr *queue);
@ -121,6 +122,7 @@ virFDStreamDataDispose(void *obj)
virFDStreamDataPtr fdst = obj; virFDStreamDataPtr fdst = obj;
VIR_DEBUG("obj=%p", fdst); VIR_DEBUG("obj=%p", fdst);
virFDStreamDataDisposed = true;
virFreeError(fdst->threadErr); virFreeError(fdst->threadErr);
virFDStreamMsgQueueFree(&fdst->msg); virFDStreamMsgQueueFree(&fdst->msg);
} }
@ -631,7 +633,9 @@ virFDStreamThread(void *opaque)
cleanup: cleanup:
fdst->threadQuit = true; fdst->threadQuit = true;
virObjectUnlock(fdst); virObjectUnlock(fdst);
if (!virObjectUnref(fdst)) virFDStreamDataDisposed = false;
virObjectUnref(fdst);
if (virFDStreamDataDisposed)
st->privateData = NULL; st->privateData = NULL;
VIR_FORCE_CLOSE(fdin); VIR_FORCE_CLOSE(fdin);
VIR_FORCE_CLOSE(fdout); VIR_FORCE_CLOSE(fdout);

View File

@ -331,17 +331,14 @@ virObjectRWLockableDispose(void *anyobj)
* it hits zero, runs the "dispose" callbacks associated * it hits zero, runs the "dispose" callbacks associated
* with the object class and its parents before freeing * with the object class and its parents before freeing
* @anyobj. * @anyobj.
*
* Returns true if the remaining reference count is
* non-zero, false if the object was disposed of
*/ */
bool void
virObjectUnref(void *anyobj) virObjectUnref(void *anyobj)
{ {
virObjectPtr obj = anyobj; virObjectPtr obj = anyobj;
if (VIR_OBJECT_NOTVALID(obj)) if (VIR_OBJECT_NOTVALID(obj))
return false; return;
bool lastRef = !!g_atomic_int_dec_and_test(&obj->u.s.refs); bool lastRef = !!g_atomic_int_dec_and_test(&obj->u.s.refs);
PROBE(OBJECT_UNREF, "obj=%p", obj); PROBE(OBJECT_UNREF, "obj=%p", obj);
@ -360,8 +357,6 @@ virObjectUnref(void *anyobj)
obj->klass = (void*)0xDEADBEEF; obj->klass = (void*)0xDEADBEEF;
VIR_FREE(obj); VIR_FREE(obj);
} }
return !lastRef;
} }

View File

@ -106,7 +106,7 @@ void *
virObjectNew(virClassPtr klass) virObjectNew(virClassPtr klass)
ATTRIBUTE_NONNULL(1); ATTRIBUTE_NONNULL(1);
bool void
virObjectUnref(void *obj); virObjectUnref(void *obj);
G_DEFINE_AUTOPTR_CLEANUP_FUNC(virObject, virObjectUnref); G_DEFINE_AUTOPTR_CLEANUP_FUNC(virObject, virObjectUnref);

View File

@ -61,6 +61,7 @@ static virClassPtr vboxDriverClass;
static virMutex vbox_driver_lock = VIR_MUTEX_INITIALIZER; static virMutex vbox_driver_lock = VIR_MUTEX_INITIALIZER;
static vboxDriverPtr vbox_driver; static vboxDriverPtr vbox_driver;
static vboxDriverPtr vboxDriverObjNew(void); static vboxDriverPtr vboxDriverObjNew(void);
static __thread bool vboxDriverDisposed;
static int static int
vboxDomainDevicesDefPostParse(virDomainDeviceDefPtr dev G_GNUC_UNUSED, vboxDomainDevicesDefPostParse(virDomainDeviceDefPtr dev G_GNUC_UNUSED,
@ -124,6 +125,7 @@ vboxDriverDispose(void *obj)
{ {
vboxDriverPtr driver = obj; vboxDriverPtr driver = obj;
vboxDriverDisposed = true;
virObjectUnref(driver->caps); virObjectUnref(driver->caps);
virObjectUnref(driver->xmlopt); virObjectUnref(driver->xmlopt);
} }
@ -250,7 +252,9 @@ vboxGetDriverConnection(void)
if (vboxSdkInitialize() < 0 || vboxExtractVersion() < 0) { if (vboxSdkInitialize() < 0 || vboxExtractVersion() < 0) {
gVBoxAPI.UPFN.Uninitialize(vbox_driver); gVBoxAPI.UPFN.Uninitialize(vbox_driver);
/* make sure to clear the pointer when last reference was released */ /* make sure to clear the pointer when last reference was released */
if (!virObjectUnref(vbox_driver)) vboxDriverDisposed = false;
virObjectUnref(vbox_driver);
if (vboxDriverDisposed)
vbox_driver = NULL; vbox_driver = NULL;
virMutexUnlock(&vbox_driver_lock); virMutexUnlock(&vbox_driver_lock);
@ -277,7 +281,9 @@ vboxDestroyDriverConnection(void)
vboxSdkUninitialize(); vboxSdkUninitialize();
if (!virObjectUnref(vbox_driver)) vboxDriverDisposed = false;
virObjectUnref(vbox_driver);
if (vboxDriverDisposed)
vbox_driver = NULL; vbox_driver = NULL;
cleanup: cleanup: