Merge branch 'g_main_context_check_skipping_pollrec_updates' into 'master'

gmain: g_main_context_check() can skip updating polled FD sources

Closes #1592

See merge request GNOME/glib!1713

glib/gmain.c

@@ -3733,7 +3733,10 @@ g_main_context_prepare (GMainContext *context,
  *       store #GPollFD records that need to be polled.
  * @n_fds: (in): length of @fds.
  *
- * Determines information necessary to poll this main loop.
+ * Determines information necessary to poll this main loop. You should
+ * be careful to pass the resulting @fds array and its length @n_fds
+ * as is when calling g_main_context_check(), as this function relies
+ * on assumptions made when the array is filled.
  *
  * You must have successfully acquired the context with
  * g_main_context_acquire() before you may call this function.
@@ -3757,6 +3760,10 @@ g_main_context_query (GMainContext *context,
 
   TRACE (GLIB_MAIN_CONTEXT_BEFORE_QUERY (context, max_priority));
 
+  /* fds is filled sequentially from poll_records. Since poll_records
+   * are incrementally sorted by file descriptor identifier, fds will
+   * also be incrementally sorted.
+   */
   n_poll = 0;
   lastpollrec = NULL;
   for (pollrec = context->poll_records; pollrec; pollrec = pollrec->next)
@@ -3771,6 +3778,10 @@ g_main_context_query (GMainContext *context,
        */
       events = pollrec->fd->events & ~(G_IO_ERR|G_IO_HUP|G_IO_NVAL);
 
+      /* This optimization --using the same GPollFD to poll for more
+       * than one poll record-- relies on the poll records being
+       * incrementally sorted.
+       */
       if (lastpollrec && pollrec->fd->fd == lastpollrec->fd->fd)
         {
           if (n_poll - 1 < n_fds)
@@ -3816,7 +3827,10 @@ g_main_context_query (GMainContext *context,
  *       the last call to g_main_context_query()
  * @n_fds: return value of g_main_context_query()
  *
- * Passes the results of polling back to the main loop.
+ * Passes the results of polling back to the main loop. You should be
+ * careful to pass @fds and its length @n_fds as received from
+ * g_main_context_query(), as this functions relies on assumptions
+ * on how @fds is filled.
  *
  * You must have successfully acquired the context with
  * g_main_context_acquire() before you may call this function.
@@ -3871,10 +3885,22 @@ g_main_context_check (GMainContext *context,
       return FALSE;
     }
 
+  /* The linear iteration below relies on the assumption that both
+   * poll records and the fds array are incrementally sorted by file
+   * descriptor identifier.
+   */
   pollrec = context->poll_records;
   i = 0;
   while (pollrec && i < n_fds)
     {
+      /* Make sure that fds is sorted by file descriptor identifier. */
+      g_assert (i <= 0 || fds[i - 1].fd < fds[i].fd);
+
+      /* Skip until finding the first GPollRec matching the current GPollFD. */
+      while (pollrec && pollrec->fd->fd != fds[i].fd)
+        pollrec = pollrec->next;
+
+      /* Update all consecutive GPollRecs that match. */
       while (pollrec && pollrec->fd->fd == fds[i].fd)
         {
           if (pollrec->priority <= max_priority)
@@ -3885,6 +3911,7 @@ g_main_context_check (GMainContext *context,
           pollrec = pollrec->next;
         }
 
+      /* Iterate to next GPollFD. */
       i++;
     }
 
@@ -4495,6 +4522,7 @@ g_main_context_add_poll_unlocked (GMainContext *context,
   newrec->fd = fd;
   newrec->priority = priority;
 
+  /* Poll records are incrementally sorted by file descriptor identifier. */
   prevrec = NULL;
   nextrec = context->poll_records;
   while (nextrec)

glib/tests/mainloop.c

@@ -1542,6 +1542,62 @@ test_unix_file_poll (void)
   close (fd);
 }
 
+static void
+test_unix_fd_priority (void)
+{
+  gint fd1, fd2;
+  GMainLoop *loop;
+  GSource *source;
+
+  gint s1 = 0;
+  gboolean s2 = FALSE, s3 = FALSE;
+
+  g_test_bug ("https://gitlab.gnome.org/GNOME/glib/-/issues/1592");
+
+  loop = g_main_loop_new (NULL, FALSE);
+
+  source = g_idle_source_new ();
+  g_source_set_callback (source, count_calls, &s1, NULL);
+  g_source_set_priority (source, 0);
+  g_source_attach (source, NULL);
+  g_source_unref (source);
+
+  fd1 = open ("/dev/random", O_RDONLY);
+  g_assert_cmpint (fd1, >=, 0);
+  source = g_unix_fd_source_new (fd1, G_IO_IN);
+  g_source_set_callback (source, G_SOURCE_FUNC (flag_bool), &s2, NULL);
+  g_source_set_priority (source, 10);
+  g_source_attach (source, NULL);
+  g_source_unref (source);
+
+  fd2 = open ("/dev/random", O_RDONLY);
+  g_assert_cmpint (fd2, >=, 0);
+  source = g_unix_fd_source_new (fd2, G_IO_IN);
+  g_source_set_callback (source, G_SOURCE_FUNC (flag_bool), &s3, NULL);
+  g_source_set_priority (source, 0);
+  g_source_attach (source, NULL);
+  g_source_unref (source);
+
+  /* This tests a bug that depends on the source with the lowest FD
+     identifier to have the lowest priority. Make sure that this is
+     the case. */
+  g_assert_cmpint (fd1, <, fd2);
+
+  g_assert_true (g_main_context_iteration (NULL, FALSE));
+
+  /* Idle source should have been dispatched. */
+  g_assert_cmpint (s1, ==, 1);
+  /* Low priority FD source shouldn't have been dispatched. */
+  g_assert_false (s2);
+  /* Default priority FD source should have been dispatched. */
+  g_assert_true (s3);
+
+  g_main_loop_unref (loop);
+
+  close (fd1);
+  close (fd2);
+}
+
 #endif
 
 #ifdef G_OS_UNIX
@@ -2035,6 +2091,7 @@ main (int argc, char *argv[])
   g_test_add_func ("/mainloop/source-unix-fd-api", test_source_unix_fd_api);
   g_test_add_func ("/mainloop/wait", test_mainloop_wait);
   g_test_add_func ("/mainloop/unix-file-poll", test_unix_file_poll);
+  g_test_add_func ("/mainloop/unix-fd-priority", test_unix_fd_priority);
 #endif
   g_test_add_func ("/mainloop/nfds", test_nfds);