The red_worker EventListener struct is either embedded in one of:
1) DisplayChannelClient
2) CursorChannelClient
3) RedWorker
And as such gets destroyed when these get destroyed, in case 1 & 2 through
a call to red_channel_client_destroy().
So free-ing it when the ref-count becomes 0 is wrong, for cases:
1) and 2) this will lead to a double free;
3) this will lead to passing memory to free which was not returned by malloc.
This is not causing any issues as the ref-count never gets decremented, other
then in red_worker_main where it gets incremented before it gets decremented,
so it never becomes 0.
So we might just as well completely remove it.
Notes:
1) This is mainly a preparation patch for fixing issues introduced by
the move from epoll to poll
2) Since removing the ref-counting removes the one code path where listeners
would get set to NULL, this patch moves the setting of NULL to
pre_disconnect, where it should have been done in the first place since
red_client_destroy calls red_channel_client_disconnect
(through the dispatcher) followed by red_channel_client_destroy, so
after pre_disconnect the listener may be gone.
Signed-off-by: Hans de Goede <hdego...@redhat.com>
---
server/red_worker.c | 36 ++----------------------------------
1 file changed, 2 insertions(+), 34 deletions(-)
diff --git a/server/red_worker.c b/server/red_worker.c
index 6de95cb..3da5161 100644
--- a/server/red_worker.c
+++ b/server/red_worker.c
@@ -237,11 +237,8 @@ double inline stat_byte_to_mega(uint64_t size)
typedef struct EventListener EventListener;
typedef void (*event_listener_action_proc)(EventListener *ctx, struct pollfd
*pfd);
-typedef void (*event_listener_free_proc)(EventListener *ctx);
struct EventListener {
- uint32_t refs;
event_listener_action_proc action;
- event_listener_free_proc free;
};
enum {
@@ -8732,6 +8729,7 @@ static void
poll_channel_client_pre_disconnect(RedChannelClient *rcc)
struct pollfd *pfd = common->worker->poll_fds + i;
if (pfd->fd == rcc->stream->socket) {
pfd->fd = -1;
+ common->worker->listeners[i] = NULL;
break;
}
}
@@ -9534,14 +9532,6 @@ static int common_channel_config_socket(RedChannelClient
*rcc)
return TRUE;
}
-static void free_common_cc_from_listener(EventListener *ctx)
-{
- CommonChannelClient* common_cc = SPICE_CONTAINEROF(ctx,
CommonChannelClient, listener);
-
- red_printf("");
- free(common_cc);
-}
-
static void worker_watch_update_mask(SpiceWatch *watch, int event_mask)
{
}
@@ -9630,9 +9620,7 @@ static int listen_to_new_client_channel(CommonChannel
*common,
{
int i;
- common_cc->listener.refs = 1;
common_cc->listener.action = common->listener_action;
- common_cc->listener.free = free_common_cc_from_listener;
ASSERT(common->base.clients_num);
common_cc->id = common->worker->id;
red_printf("NEW ID = %d", common_cc->id);
@@ -11059,11 +11047,6 @@ static void handle_dev_input(EventListener *listener,
struct pollfd *pfd)
dispatcher_handle_recv_read(red_dispatcher_get_dispatcher(worker->red_dispatcher));
}
-static void handle_dev_free(EventListener *ctx)
-{
- free(ctx);
-}
-
static void red_init(RedWorker *worker, WorkerInitData *init_data)
{
RedWorkerMessage message;
@@ -11081,9 +11064,7 @@ static void red_init(RedWorker *worker, WorkerInitData
*init_data)
worker->channel = dispatcher_get_recv_fd(dispatcher);
register_callbacks(dispatcher);
worker->pending = init_data->pending;
- worker->dev_listener.refs = 1;
worker->dev_listener.action = handle_dev_input;
- worker->dev_listener.free = handle_dev_free;
worker->cursor_visible = TRUE;
ASSERT(init_data->num_renderers > 0);
worker->num_renderers = init_data->num_renderers;
@@ -11185,24 +11166,11 @@ void *red_worker_main(void *arg)
for (i = 0; i < MAX_EVENT_SOURCES; i++) {
struct pollfd *pfd = worker.poll_fds + i;
if (pfd->revents) {
- worker.listeners[i]->refs++;
- }
- }
-
- for (i = 0; i < MAX_EVENT_SOURCES; i++) {
- struct pollfd *pfd = worker.poll_fds + i;
- if (pfd->revents) {
EventListener *evt_listener = worker.listeners[i];
- if (evt_listener && evt_listener->refs > 1) {
+ if (evt_listener) {
evt_listener->action(evt_listener, pfd);
- if (--evt_listener->refs) {
- continue;
- }
}
- red_printf("freeing event listener");
- evt_listener->free(evt_listener);
- worker.listeners[i] = NULL;
}
}
--
1.7.9.3
_______________________________________________
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel
