diff --git a/glib/gmain.c b/glib/gmain.c index 47e7749a3..e2b6ca3b1 100644 --- a/glib/gmain.c +++ b/glib/gmain.c @@ -249,7 +249,7 @@ struct _GMainContext GList *source_lists; gint in_check_or_prepare; - GPollRec *poll_records, *poll_records_tail; + GPollRec *poll_records; guint n_poll_records; GPollFD *cached_poll_array; guint cached_poll_array_size; @@ -3506,33 +3506,43 @@ g_main_context_query (GMainContext *context, gint n_fds) { gint n_poll; - GPollRec *pollrec; + GPollRec *pollrec, *lastpollrec; + gushort events; LOCK_CONTEXT (context); - pollrec = context->poll_records; n_poll = 0; - while (pollrec && max_priority >= pollrec->priority) + lastpollrec = NULL; + for (pollrec = context->poll_records; pollrec; pollrec = pollrec->next) { - /* We need to include entries with fd->events == 0 in the array because - * otherwise if the application changes fd->events behind our back and - * makes it non-zero, we'll be out of sync when we check the fds[] array. - * (Changing fd->events after adding an FD wasn't an anticipated use of - * this API, but it occurs in practice.) */ - if (n_poll < n_fds) - { - fds[n_poll].fd = pollrec->fd->fd; - /* In direct contradiction to the Unix98 spec, IRIX runs into - * difficulty if you pass in POLLERR, POLLHUP or POLLNVAL - * flags in the events field of the pollfd while it should - * just ignoring them. So we mask them out here. - */ - fds[n_poll].events = pollrec->fd->events & ~(G_IO_ERR|G_IO_HUP|G_IO_NVAL); - fds[n_poll].revents = 0; - } + if (pollrec->priority > max_priority) + continue; - pollrec = pollrec->next; - n_poll++; + /* In direct contradiction to the Unix98 spec, IRIX runs into + * difficulty if you pass in POLLERR, POLLHUP or POLLNVAL + * flags in the events field of the pollfd while it should + * just ignoring them. So we mask them out here. + */ + events = pollrec->fd->events & ~(G_IO_ERR|G_IO_HUP|G_IO_NVAL); + + if (lastpollrec && pollrec->fd->fd == lastpollrec->fd->fd) + { + if (n_poll - 1 < n_fds) + fds[n_poll - 1].events |= events; + } + else + { + if (n_poll < n_fds) + { + fds[n_poll].fd = pollrec->fd->fd; + fds[n_poll].events = events; + fds[n_poll].revents = 0; + } + + n_poll++; + } + + lastpollrec = pollrec; } context->poll_changed = FALSE; @@ -3597,15 +3607,21 @@ g_main_context_check (GMainContext *context, UNLOCK_CONTEXT (context); return FALSE; } - + pollrec = context->poll_records; i = 0; - while (i < n_fds) + while (pollrec && i < n_fds) { - if (pollrec->fd->events) - pollrec->fd->revents = fds[i].revents; + while (pollrec && pollrec->fd->fd == fds[i].fd) + { + if (pollrec->priority <= max_priority) + { + pollrec->fd->revents = + fds[i].revents & (pollrec->fd->events | G_IO_ERR | G_IO_HUP | G_IO_NVAL); + } + pollrec = pollrec->next; + } - pollrec = pollrec->next; i++; } @@ -4185,12 +4201,14 @@ g_main_context_add_poll_unlocked (GMainContext *context, newrec->fd = fd; newrec->priority = priority; - prevrec = context->poll_records_tail; - nextrec = NULL; - while (prevrec && priority < prevrec->priority) + prevrec = NULL; + nextrec = context->poll_records; + while (nextrec) { - nextrec = prevrec; - prevrec = prevrec->prev; + if (nextrec->fd > fd) + break; + prevrec = nextrec; + nextrec = nextrec->next; } if (prevrec) @@ -4203,8 +4221,6 @@ g_main_context_add_poll_unlocked (GMainContext *context, if (nextrec) nextrec->prev = newrec; - else - context->poll_records_tail = newrec; context->n_poll_records++; @@ -4258,8 +4274,6 @@ g_main_context_remove_poll_unlocked (GMainContext *context, if (nextrec != NULL) nextrec->prev = prevrec; - else - context->poll_records_tail = prevrec; g_slice_free (GPollRec, pollrec); diff --git a/glib/gpoll.c b/glib/gpoll.c index 2620c9a90..adc67820a 100644 --- a/glib/gpoll.c +++ b/glib/gpoll.c @@ -271,30 +271,17 @@ g_poll (GPollFD *fds, } else if (f->fd > 0) { - /* Don't add the same handle several times into the array, as - * docs say that is not allowed, even if it actually does seem - * to work. - */ - gint i; - - for (i = 0; i < nhandles; i++) - if (handles[i] == (HANDLE) f->fd) - break; - - if (i == nhandles) - { - if (nhandles == MAXIMUM_WAIT_OBJECTS) - { - g_warning ("Too many handles to wait for!\n"); - break; - } - else - { - if (_g_main_poll_debug) - g_print (" %p", (HANDLE) f->fd); - handles[nhandles++] = (HANDLE) f->fd; - } - } + if (nhandles == MAXIMUM_WAIT_OBJECTS) + { + g_warning ("Too many handles to wait for!\n"); + break; + } + else + { + if (_g_main_poll_debug) + g_print (" %p", (HANDLE) f->fd); + handles[nhandles++] = (HANDLE) f->fd; + } } if (_g_main_poll_debug) diff --git a/glib/tests/mainloop.c b/glib/tests/mainloop.c index 8abd262f1..f5d672a63 100644 --- a/glib/tests/mainloop.c +++ b/glib/tests/mainloop.c @@ -22,6 +22,7 @@ #include #include "glib-private.h" +#include #include static gboolean cb (gpointer data) @@ -1564,6 +1565,165 @@ test_mainloop_wait (void) g_main_context_unref (context); } +static gboolean +nfds_in_cb (GIOChannel *io, + GIOCondition condition, + gpointer user_data) +{ + gboolean *in_cb_ran = user_data; + + *in_cb_ran = TRUE; + g_assert_cmpint (condition, ==, G_IO_IN); + return FALSE; +} + +static gboolean +nfds_out_cb (GIOChannel *io, + GIOCondition condition, + gpointer user_data) +{ + gboolean *out_cb_ran = user_data; + + *out_cb_ran = TRUE; + g_assert_cmpint (condition, ==, G_IO_OUT); + return FALSE; +} + +static gboolean +nfds_out_low_cb (GIOChannel *io, + GIOCondition condition, + gpointer user_data) +{ + g_assert_not_reached (); + return FALSE; +} + +static void +test_nfds (void) +{ + GMainContext *ctx; + GPollFD out_fds[3]; + gint fd, nfds; + GIOChannel *io; + GSource *source1, *source2, *source3; + gboolean source1_ran = FALSE, source3_ran = FALSE; + gchar *tmpfile; + GError *error = NULL; + + ctx = g_main_context_new (); + nfds = g_main_context_query (ctx, G_MAXINT, NULL, + out_fds, G_N_ELEMENTS (out_fds)); + /* An "empty" GMainContext will have a single GPollFD, for its + * internal GWakeup. + */ + g_assert_cmpint (nfds, ==, 1); + + fd = g_file_open_tmp (NULL, &tmpfile, &error); + g_assert_no_error (error); + + io = g_io_channel_unix_new (fd); +#ifdef G_OS_WIN32 + /* The fd in the pollfds won't be the same fd we passed in */ + g_io_channel_win32_make_pollfd (io, G_IO_IN, out_fds); + fd = out_fds[0].fd; +#endif + + /* Add our first pollfd */ + source1 = g_io_create_watch (io, G_IO_IN); + g_source_set_priority (source1, G_PRIORITY_DEFAULT); + g_source_set_callback (source1, (GSourceFunc) nfds_in_cb, + &source1_ran, NULL); + g_source_attach (source1, ctx); + + nfds = g_main_context_query (ctx, G_MAXINT, NULL, + out_fds, G_N_ELEMENTS (out_fds)); + g_assert_cmpint (nfds, ==, 2); + if (out_fds[0].fd == fd) + g_assert_cmpint (out_fds[0].events, ==, G_IO_IN); + else if (out_fds[1].fd == fd) + g_assert_cmpint (out_fds[1].events, ==, G_IO_IN); + else + g_assert_not_reached (); + + /* Add a second pollfd with the same fd but different event, and + * lower priority. + */ + source2 = g_io_create_watch (io, G_IO_OUT); + g_source_set_priority (source2, G_PRIORITY_LOW); + g_source_set_callback (source2, (GSourceFunc) nfds_out_low_cb, + NULL, NULL); + g_source_attach (source2, ctx); + + /* g_main_context_query() should still return only 2 pollfds, + * one of which has our fd, and a combined events field. + */ + nfds = g_main_context_query (ctx, G_MAXINT, NULL, + out_fds, G_N_ELEMENTS (out_fds)); + g_assert_cmpint (nfds, ==, 2); + if (out_fds[0].fd == fd) + g_assert_cmpint (out_fds[0].events, ==, G_IO_IN | G_IO_OUT); + else if (out_fds[1].fd == fd) + g_assert_cmpint (out_fds[1].events, ==, G_IO_IN | G_IO_OUT); + else + g_assert_not_reached (); + + /* But if we query with a max priority, we won't see the + * lower-priority one. + */ + nfds = g_main_context_query (ctx, G_PRIORITY_DEFAULT, NULL, + out_fds, G_N_ELEMENTS (out_fds)); + g_assert_cmpint (nfds, ==, 2); + if (out_fds[0].fd == fd) + g_assert_cmpint (out_fds[0].events, ==, G_IO_IN); + else if (out_fds[1].fd == fd) + g_assert_cmpint (out_fds[1].events, ==, G_IO_IN); + else + g_assert_not_reached (); + + /* Third pollfd */ + source3 = g_io_create_watch (io, G_IO_OUT); + g_source_set_priority (source3, G_PRIORITY_DEFAULT); + g_source_set_callback (source3, (GSourceFunc) nfds_out_cb, + &source3_ran, NULL); + g_source_attach (source3, ctx); + + nfds = g_main_context_query (ctx, G_MAXINT, NULL, + out_fds, G_N_ELEMENTS (out_fds)); + g_assert_cmpint (nfds, ==, 2); + if (out_fds[0].fd == fd) + g_assert_cmpint (out_fds[0].events, ==, G_IO_IN | G_IO_OUT); + else if (out_fds[1].fd == fd) + g_assert_cmpint (out_fds[1].events, ==, G_IO_IN | G_IO_OUT); + else + g_assert_not_reached (); + + /* Now actually iterate the loop; the fd should be readable and + * writable, so source1 and source3 should be triggered, but *not* + * source2, since it's lower priority than them. (Though on + * G_OS_WIN32, source3 doesn't get triggered, probably because of + * giowin32 weirdness...) + */ + g_main_context_iteration (ctx, FALSE); + + g_assert (source1_ran); +#ifndef G_OS_WIN32 + g_assert (source3_ran); +#endif + + g_source_destroy (source1); + g_source_unref (source1); + g_source_destroy (source2); + g_source_unref (source2); + g_source_destroy (source3); + g_source_unref (source3); + + g_io_channel_unref (io); + remove (tmpfile); + g_free (tmpfile); + + g_main_context_unref (ctx); +} + int main (int argc, char *argv[]) { @@ -1592,6 +1752,7 @@ main (int argc, char *argv[]) g_test_add_func ("/mainloop/wait", test_mainloop_wait); g_test_add_func ("/mainloop/unix-file-poll", test_unix_file_poll); #endif + g_test_add_func ("/mainloop/nfds", test_nfds); return g_test_run (); }