util: keep track of full GSource object not source ID number

The source ID number is an alternative way to identify a source that has
been added to a GMainContext. Internally when a source ID is given, glib
will lookup the corresponding GSource and use that. The use of a source
ID is racy in some cases though, because it is invalid to continue to
use an ID number after the GSource has been removed. It is thus safer
to use the GSource object directly and have full control over the ref
counting and thus cleanup.

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-07-28 15:54:13 +01:00
parent 84bb5fd1ab
commit da0a182708
4 changed files with 79 additions and 53 deletions

View File

@ -826,6 +826,7 @@ virNetClientIOEventTLS(int fd,
static gboolean static gboolean
virNetClientTLSHandshake(virNetClientPtr client) virNetClientTLSHandshake(virNetClientPtr client)
{ {
g_autoptr(GSource) source = NULL;
GIOCondition ev; GIOCondition ev;
int ret; int ret;
@ -840,7 +841,7 @@ virNetClientTLSHandshake(virNetClientPtr client)
else else
ev = G_IO_OUT; ev = G_IO_OUT;
virEventGLibAddSocketWatch(virNetSocketGetFD(client->sock), source = virEventGLibAddSocketWatch(virNetSocketGetFD(client->sock),
ev, ev,
client->eventCtx, client->eventCtx,
virNetClientIOEventTLS, client, NULL); virNetClientIOEventTLS, client, NULL);
@ -882,6 +883,7 @@ int virNetClientSetTLSSession(virNetClientPtr client,
int ret; int ret;
char buf[1]; char buf[1];
int len; int len;
g_autoptr(GSource) source = NULL;
#ifndef WIN32 #ifndef WIN32
sigset_t oldmask, blockedsigs; sigset_t oldmask, blockedsigs;
@ -934,7 +936,7 @@ int virNetClientSetTLSSession(virNetClientPtr client,
* etc. If we make the grade, it will send us a '\1' byte. * etc. If we make the grade, it will send us a '\1' byte.
*/ */
virEventGLibAddSocketWatch(virNetSocketGetFD(client->sock), source = virEventGLibAddSocketWatch(virNetSocketGetFD(client->sock),
G_IO_IN, G_IO_IN,
client->eventCtx, client->eventCtx,
virNetClientIOEventTLSConfirm, client, NULL); virNetClientIOEventTLSConfirm, client, NULL);
@ -1617,6 +1619,7 @@ static int virNetClientIOEventLoop(virNetClientPtr client,
#endif /* !WIN32 */ #endif /* !WIN32 */
int timeout = -1; int timeout = -1;
virNetMessagePtr msg = NULL; virNetMessagePtr msg = NULL;
g_autoptr(GSource) source = NULL;
GIOCondition ev = 0; GIOCondition ev = 0;
struct virNetClientIOEventData data = { struct virNetClientIOEventData data = {
.client = client, .client = client,
@ -1651,7 +1654,7 @@ static int virNetClientIOEventLoop(virNetClientPtr client,
if (client->nstreams) if (client->nstreams)
ev |= G_IO_IN; ev |= G_IO_IN;
virEventGLibAddSocketWatch(virNetSocketGetFD(client->sock), source = virEventGLibAddSocketWatch(virNetSocketGetFD(client->sock),
ev, ev,
client->eventCtx, client->eventCtx,
virNetClientIOEventFD, &data, NULL); virNetClientIOEventFD, &data, NULL);

View File

@ -45,7 +45,7 @@ struct virEventGLibHandle
int fd; int fd;
int events; int events;
int removed; int removed;
guint source; GSource *source;
virEventHandleCallback cb; virEventHandleCallback cb;
void *opaque; void *opaque;
virFreeCallback ff; virFreeCallback ff;
@ -56,7 +56,7 @@ struct virEventGLibTimeout
int timer; int timer;
int interval; int interval;
int removed; int removed;
guint source; GSource *source;
virEventTimeoutCallback cb; virEventTimeoutCallback cb;
void *opaque; void *opaque;
virFreeCallback ff; virFreeCallback ff;
@ -210,23 +210,25 @@ virEventGLibHandleUpdate(int watch,
if (events == data->events) if (events == data->events)
goto cleanup; goto cleanup;
if (data->source != 0) { if (data->source != NULL) {
VIR_DEBUG("Removed old handle watch=%d", data->source); VIR_DEBUG("Removed old handle source=%p", data->source);
g_source_remove(data->source); g_source_destroy(data->source);
g_source_unref(data->source);
} }
data->source = virEventGLibAddSocketWatch( data->source = virEventGLibAddSocketWatch(
data->fd, cond, NULL, virEventGLibHandleDispatch, data, NULL); data->fd, cond, NULL, virEventGLibHandleDispatch, data, NULL);
data->events = events; data->events = events;
VIR_DEBUG("Added new handle watch=%d", data->source); VIR_DEBUG("Added new handle source=%p", data->source);
} else { } else {
if (data->source == 0) if (data->source == NULL)
goto cleanup; goto cleanup;
VIR_DEBUG("Removed old handle watch=%d", data->source); VIR_DEBUG("Removed old handle source=%p", data->source);
g_source_remove(data->source); g_source_destroy(data->source);
data->source = 0; g_source_unref(data->source);
data->source = NULL;
data->events = 0; data->events = 0;
} }
@ -272,9 +274,10 @@ virEventGLibHandleRemove(int watch)
VIR_DEBUG("Remove handle data=%p watch=%d fd=%d", VIR_DEBUG("Remove handle data=%p watch=%d fd=%d",
data, watch, data->fd); data, watch, data->fd);
if (data->source != 0) { if (data->source != NULL) {
g_source_remove(data->source); g_source_destroy(data->source);
data->source = 0; g_source_unref(data->source);
data->source = NULL;
data->events = 0; data->events = 0;
} }
@ -309,6 +312,22 @@ virEventGLibTimeoutDispatch(void *opaque)
return TRUE; return TRUE;
} }
static GSource *
virEventGLibTimeoutCreate(int interval,
struct virEventGLibTimeout *data)
{
GSource *source = g_timeout_source_new(interval);
g_source_set_callback(source,
virEventGLibTimeoutDispatch,
data, NULL);
g_source_attach(source, NULL);
return source;
}
static int static int
virEventGLibTimeoutAdd(int interval, virEventGLibTimeoutAdd(int interval,
virEventTimeoutCallback cb, virEventTimeoutCallback cb,
@ -327,9 +346,7 @@ virEventGLibTimeoutAdd(int interval,
data->opaque = opaque; data->opaque = opaque;
data->ff = ff; data->ff = ff;
if (interval >= 0) if (interval >= 0)
data->source = g_timeout_add(interval, data->source = virEventGLibTimeoutCreate(interval, data);
virEventGLibTimeoutDispatch,
data);
g_ptr_array_add(timeouts, data); g_ptr_array_add(timeouts, data);
@ -390,19 +407,20 @@ virEventGLibTimeoutUpdate(int timer,
VIR_DEBUG("Update timeout data=%p timer=%d interval=%d ms", data, timer, interval); VIR_DEBUG("Update timeout data=%p timer=%d interval=%d ms", data, timer, interval);
if (interval >= 0) { if (interval >= 0) {
if (data->source != 0) if (data->source != NULL) {
g_source_remove(data->source); g_source_destroy(data->source);
g_source_unref(data->source);
}
data->interval = interval; data->interval = interval;
data->source = g_timeout_add(data->interval, data->source = virEventGLibTimeoutCreate(interval, data);
virEventGLibTimeoutDispatch,
data);
} else { } else {
if (data->source == 0) if (data->source == NULL)
goto cleanup; goto cleanup;
g_source_remove(data->source); g_source_destroy(data->source);
data->source = 0; g_source_unref(data->source);
data->source = NULL;
} }
cleanup: cleanup:
@ -448,9 +466,10 @@ virEventGLibTimeoutRemove(int timer)
VIR_DEBUG("Remove timeout data=%p timer=%d", VIR_DEBUG("Remove timeout data=%p timer=%d",
data, timer); data, timer);
if (data->source != 0) { if (data->source != NULL) {
g_source_remove(data->source); g_source_destroy(data->source);
data->source = 0; g_source_unref(data->source);
data->source = NULL;
} }
/* since the actual timeout deletion is done asynchronously, a timeoutUpdate call may /* since the actual timeout deletion is done asynchronously, a timeoutUpdate call may

View File

@ -233,17 +233,20 @@ GSource *virEventGLibCreateSocketWatch(int fd,
#endif /* WIN32 */ #endif /* WIN32 */
guint virEventGLibAddSocketWatch(int fd, GSource *
virEventGLibAddSocketWatch(int fd,
GIOCondition condition, GIOCondition condition,
GMainContext *context, GMainContext *context,
virEventGLibSocketFunc func, virEventGLibSocketFunc func,
gpointer opaque, gpointer opaque,
GDestroyNotify notify) GDestroyNotify notify)
{ {
g_autoptr(GSource) source = NULL; GSource *source = NULL;
source = virEventGLibCreateSocketWatch(fd, condition); source = virEventGLibCreateSocketWatch(fd, condition);
g_source_set_callback(source, (GSourceFunc)func, opaque, notify); g_source_set_callback(source, (GSourceFunc)func, opaque, notify);
return g_source_attach(source, context); g_source_attach(source, context);
return source;
} }

View File

@ -40,9 +40,10 @@ typedef gboolean (*virEventGLibSocketFunc)(int fd,
GIOCondition condition, GIOCondition condition,
gpointer data); gpointer data);
guint virEventGLibAddSocketWatch(int fd, GSource *virEventGLibAddSocketWatch(int fd,
GIOCondition condition, GIOCondition condition,
GMainContext *context, GMainContext *context,
virEventGLibSocketFunc func, virEventGLibSocketFunc func,
gpointer opaque, gpointer opaque,
GDestroyNotify notify); GDestroyNotify notify)
G_GNUC_WARN_UNUSED_RESULT;