diff options
author | Hans de Goede <hdegoede@redhat.com> | 2012-03-10 12:06:39 +0100 |
---|---|---|
committer | Hans de Goede <hdegoede@redhat.com> | 2012-03-10 13:56:29 +0100 |
commit | 63e1514ccbcdbf122c4f61f5a9511db586b7ae0d (patch) | |
tree | 72f8fb74a530d9d4c19d745acbd8f37453160412 /server | |
parent | f24203e122c756e3b3ee540c718409451e4f9458 (diff) | |
download | spice-63e1514ccbcdbf122c4f61f5a9511db586b7ae0d.tar.gz spice-63e1514ccbcdbf122c4f61f5a9511db586b7ae0d.tar.xz spice-63e1514ccbcdbf122c4f61f5a9511db586b7ae0d.zip |
red_worker: Remove ref counting from the EventListener struct
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 <hdegoede@redhat.com>
Diffstat (limited to 'server')
-rw-r--r-- | server/red_worker.c | 36 |
1 files changed, 2 insertions, 34 deletions
diff --git a/server/red_worker.c b/server/red_worker.c index 6de95cb8..3da51612 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; } } |