[Spice-devel] [PATCH v3 5/6] Do not use a mix of g_mail_loop and basic_loop in replay

[Marc-André Lureau]

Hi

----- Original Message -----
> Utility was not working due to strange mixing of these two loops

basic main loop is just:

void basic_event_loop_mainloop(void)
{
    GMainLoop *loop = g_main_loop_new(NULL, FALSE);

    g_main_loop_run(loop);
    g_main_loop_unref(loop);
}


So what's the difference except hiding the fact that gmainloop is being used,
while not being accessible, making things unnecessarily more difficult?
What didn't work?

> Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
> ---
>  server/tests/replay.c | 104
>  +++++++++++++++++++++++---------------------------
>  1 file changed, 47 insertions(+), 57 deletions(-)
> 
> diff --git a/server/tests/replay.c b/server/tests/replay.c
> index 01590c0..b55f19a 100644
> --- a/server/tests/replay.c
> +++ b/server/tests/replay.c
> @@ -12,6 +12,7 @@
>  #include <sys/wait.h>
>  #include <fcntl.h>
>  #include <glib.h>
> +#include <glib-unix.h>
>  
>  #include <spice/macros.h>
>  #include "red_replay_qxl.h"
> @@ -26,12 +27,9 @@ static gboolean started = FALSE;
>  static QXLInstance display_sin = { 0, };
>  static gint slow = 0;
>  static pid_t client_pid;
> -static GMainLoop *loop = NULL;
>  static GAsyncQueue *aqueue = NULL;
>  static long total_size;
> -
> -static GMutex mutex;
> -static guint fill_source_id = 0;
> +static int close_pipe = -1;
>  
>  
>  #define MEM_SLOT_GROUP_ID 0
> @@ -83,48 +81,37 @@ static void get_init_info(QXLInstance *qin,
> QXLDevInitInfo *info)
>      info->n_surfaces = MAX_SURFACE_NUM;
>  }
>  
> -static gboolean fill_queue_idle(gpointer user_data)
> -{
> -    gboolean keep = FALSE;
> -
> -    while (g_async_queue_length(aqueue) < 50) {
> -        QXLCommandExt *cmd = spice_replay_next_cmd(replay, qxl_worker);
> -        if (!cmd) {
> -            g_async_queue_push(aqueue, GINT_TO_POINTER(-1));
> -            goto end;
> -        }
> -
> -        if (slow)
> -            g_usleep(slow);
> -
> -        g_async_queue_push(aqueue, cmd);
> -    }
>  
> -end:
> -    if (!keep) {
> -        g_mutex_lock(&mutex);
> -        fill_source_id = 0;
> -        g_mutex_unlock(&mutex);
> -    }
> -    spice_qxl_wakeup(&display_sin);
> -
> -    return keep;
> -}
> -
> -static void fill_queue(void)
> +static gboolean fill_command(void)
>  {
> -    g_mutex_lock(&mutex);
> +    QXLCommandExt *cmd = spice_replay_next_cmd(replay, qxl_worker);
>  
> -    if (!started)
> -        goto end;
> +    g_async_queue_push(aqueue, cmd ? cmd : GINT_TO_POINTER(-1));
>  
> -    if (fill_source_id != 0)
> -        goto end;
> +    return cmd ? TRUE : FALSE;
> +}
>  
> -    fill_source_id = g_idle_add(fill_queue_idle, NULL);
>  
> -end:
> -    g_mutex_unlock(&mutex);
> +static void fill_queue_slow(void *opaque)
> +{
> +    if (slow) {
> +        if (fill_command()) {
> +            SpiceTimer *timer = core->timer_add(fill_queue_slow, NULL);
> +            core->timer_start(timer, slow);
> +        }
> +        spice_qxl_wakeup(&display_sin);
> +    } else {
> +        gboolean got_some = FALSE;
> +        while (g_async_queue_length(aqueue) < 50) {
> +            if (!fill_command())
> +                return;
> +            got_some = TRUE;
> +        }
> +        SpiceTimer *timer = core->timer_add(fill_queue_slow, NULL);
> +        core->timer_start(timer, 1);
> +        if (got_some)
> +            spice_qxl_wakeup(&display_sin);
> +    }
>  }
>  
>  
> @@ -133,15 +120,12 @@ static int get_command(QXLInstance *qin, QXLCommandExt
> *ext)
>  {
>      QXLCommandExt *cmd;
>  
> -    if (g_async_queue_length(aqueue) == 0) {
> -        /* could use a gcondition ? */
> -        fill_queue();
> +    cmd = g_async_queue_try_pop(aqueue);
> +    if (!cmd)
>          return FALSE;
> -    }
>  
> -    cmd = g_async_queue_try_pop(aqueue);
>      if (GPOINTER_TO_INT(cmd) == -1) {
> -        g_main_loop_quit(loop);
> +        write(close_pipe, "bye", 3);
>          return FALSE;
>      }
>  
> @@ -155,8 +139,8 @@ static int req_cmd_notification(QXLInstance *qin)
>      if (!started)
>          return TRUE;
>  
> -    g_printerr("id: %d, queue length: %d",
> -                   fill_source_id, g_async_queue_length(aqueue));
> +    g_printerr("queue length: %d",
> +                   g_async_queue_length(aqueue));
>  
>      return TRUE;
>  }
> @@ -257,6 +241,11 @@ static gboolean progress_timer(gpointer user_data)
>      return TRUE;
>  }
>  
> +static void close_pipe_read(int fd, int event, void *opaque)
> +{
> +    end_replay();
> +}
> +
>  int main(int argc, char **argv)
>  {
>      GError *error = NULL;
> @@ -265,6 +254,7 @@ int main(int argc, char **argv)
>      gint port = 5000, compression = SPICE_IMAGE_COMPRESSION_AUTO_GLZ;
>      gboolean wait = FALSE;
>      FILE *fd;
> +    gint pipe[2];
>  
>      GOptionEntry entries[] = {
>          { "client", 'c', 0, G_OPTION_ARG_STRING, &client, "Client", "CMD" },
> @@ -311,6 +301,13 @@ int main(int argc, char **argv)
>      core = basic_event_loop_init();
>      core->channel_event = replay_channel_event;
>  
> +    if (!g_unix_open_pipe(pipe, FD_CLOEXEC, &error)) {
> +        g_printerr("Error creating pipe: %s\n", error->message);
> +        exit(1);
> +    }
> +    close_pipe = pipe[1];
> +    core->watch_add(pipe[0], SPICE_WATCH_EVENT_READ, close_pipe_read, NULL);
> +
>      server = spice_server_new();
>      spice_server_set_image_compression(server, compression);
>      spice_server_set_port(server, port);
> @@ -329,18 +326,11 @@ int main(int argc, char **argv)
>  
>      if (!wait) {
>          started = TRUE;
> -        fill_queue();
>      }
>  
> -    loop = g_main_loop_new(NULL, FALSE);
> -    g_main_loop_run(loop);
> -
> -    end_replay();
> -    g_async_queue_unref(aqueue);
> +    fill_queue_slow(NULL);
>  
> -    /* FIXME: there should be a way to join server threads before:
> -     * g_main_loop_unref(loop);
> -     */
> +    basic_event_loop_mainloop();
>  
>      return 0;
>  }
> --
> 2.4.3
> 
> _______________________________________________
> Spice-devel mailing list
> Spice-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/spice-devel
>