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>
This commit is contained in:
Hans de Goede 2012-03-10 12:06:39 +01:00
parent f24203e122
commit 63e1514ccb

View File

@ -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;
@ -11182,27 +11163,14 @@ void *red_worker_main(void *arg)
num_events = 0;
}
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;
}
}