events: Propose a separate lock for event queue

Currently, push & pop from event queue (both server & client side)
rely on lock from higher levels, e.g. on driver lock (qemu),
private_data (remote), ...; This alone is not sufficient as not
every function that interacts with this queue can/does lock,
esp. in client where we have a different approach, "passing
the buck".

Therefore we need a separate lock just to protect event queue.

For more info see:
https://bugzilla.redhat.com/show_bug.cgi?id=743817
This commit is contained in:
Michal Privoznik 2011-10-06 18:44:13 +02:00
parent 2050b61dec
commit d81eee40c2
2 changed files with 47 additions and 8 deletions

View File

@ -547,6 +547,18 @@ virDomainEventQueuePtr virDomainEventQueueNew(void)
return ret; return ret;
} }
static void
virDomainEventStateLock(virDomainEventStatePtr state)
{
virMutexLock(&state->lock);
}
static void
virDomainEventStateUnlock(virDomainEventStatePtr state)
{
virMutexUnlock(&state->lock);
}
/** /**
* virDomainEventStateFree: * virDomainEventStateFree:
* @list: virDomainEventStatePtr to free * @list: virDomainEventStatePtr to free
@ -564,6 +576,8 @@ virDomainEventStateFree(virDomainEventStatePtr state)
if (state->timer != -1) if (state->timer != -1)
virEventRemoveTimeout(state->timer); virEventRemoveTimeout(state->timer);
virMutexDestroy(&state->lock);
VIR_FREE(state); VIR_FREE(state);
} }
@ -590,6 +604,13 @@ virDomainEventStateNew(virEventTimeoutCallback timeout_cb,
goto error; goto error;
} }
if (virMutexInit(&state->lock) < 0) {
virReportSystemError(errno, "%s",
_("unable to initialize state mutex"));
VIR_FREE(state);
goto error;
}
if (VIR_ALLOC(state->callbacks) < 0) { if (VIR_ALLOC(state->callbacks) < 0) {
virReportOOMError(); virReportOOMError();
goto error; goto error;
@ -1166,6 +1187,8 @@ virDomainEventStateQueue(virDomainEventStatePtr state,
return; return;
} }
virDomainEventStateLock(state);
if (virDomainEventQueuePush(state->queue, event) < 0) { if (virDomainEventQueuePush(state->queue, event) < 0) {
VIR_DEBUG("Error adding event to queue"); VIR_DEBUG("Error adding event to queue");
virDomainEventFree(event); virDomainEventFree(event);
@ -1173,6 +1196,7 @@ virDomainEventStateQueue(virDomainEventStatePtr state,
if (state->queue->count == 1) if (state->queue->count == 1)
virEventUpdateTimeout(state->timer, 0); virEventUpdateTimeout(state->timer, 0);
virDomainEventStateUnlock(state);
} }
void void
@ -1182,6 +1206,7 @@ virDomainEventStateFlush(virDomainEventStatePtr state,
{ {
virDomainEventQueue tempQueue; virDomainEventQueue tempQueue;
virDomainEventStateLock(state);
state->isDispatching = true; state->isDispatching = true;
/* Copy the queue, so we're reentrant safe when dispatchFunc drops the /* Copy the queue, so we're reentrant safe when dispatchFunc drops the
@ -1190,17 +1215,20 @@ virDomainEventStateFlush(virDomainEventStatePtr state,
tempQueue.events = state->queue->events; tempQueue.events = state->queue->events;
state->queue->count = 0; state->queue->count = 0;
state->queue->events = NULL; state->queue->events = NULL;
virEventUpdateTimeout(state->timer, -1); virEventUpdateTimeout(state->timer, -1);
virDomainEventStateUnlock(state);
virDomainEventQueueDispatch(&tempQueue, virDomainEventQueueDispatch(&tempQueue,
state->callbacks, state->callbacks,
dispatchFunc, dispatchFunc,
opaque); opaque);
/* Purge any deleted callbacks */ /* Purge any deleted callbacks */
virDomainEventStateLock(state);
virDomainEventCallbackListPurgeMarked(state->callbacks); virDomainEventCallbackListPurgeMarked(state->callbacks);
state->isDispatching = false; state->isDispatching = false;
virDomainEventStateUnlock(state);
} }
int int
@ -1208,11 +1236,16 @@ virDomainEventStateDeregister(virConnectPtr conn,
virDomainEventStatePtr state, virDomainEventStatePtr state,
virConnectDomainEventCallback callback) virConnectDomainEventCallback callback)
{ {
int ret;
virDomainEventStateLock(state);
if (state->isDispatching) if (state->isDispatching)
return virDomainEventCallbackListMarkDelete(conn, ret = virDomainEventCallbackListMarkDelete(conn,
state->callbacks, callback); state->callbacks, callback);
else else
return virDomainEventCallbackListRemove(conn, state->callbacks, callback); ret = virDomainEventCallbackListRemove(conn, state->callbacks, callback);
virDomainEventStateUnlock(state);
return ret;
} }
int int
@ -1220,10 +1253,15 @@ virDomainEventStateDeregisterAny(virConnectPtr conn,
virDomainEventStatePtr state, virDomainEventStatePtr state,
int callbackID) int callbackID)
{ {
int ret;
virDomainEventStateLock(state);
if (state->isDispatching) if (state->isDispatching)
return virDomainEventCallbackListMarkDeleteID(conn, ret = virDomainEventCallbackListMarkDeleteID(conn,
state->callbacks, callbackID); state->callbacks, callbackID);
else else
return virDomainEventCallbackListRemoveID(conn, ret = virDomainEventCallbackListRemoveID(conn,
state->callbacks, callbackID); state->callbacks, callbackID);
virDomainEventStateUnlock(state);
return ret;
} }

View File

@ -61,6 +61,7 @@ struct _virDomainEventState {
int timer; int timer;
/* Flag if we're in process of dispatching */ /* Flag if we're in process of dispatching */
bool isDispatching; bool isDispatching;
virMutex lock;
}; };
typedef struct _virDomainEventState virDomainEventState; typedef struct _virDomainEventState virDomainEventState;
typedef virDomainEventState *virDomainEventStatePtr; typedef virDomainEventState *virDomainEventStatePtr;