Attachment 42380 Details for Bug 33342 – [7/11] DBusLoop: keep separate lists of watches and timeouts

[patch] [7/11] DBusLoop: keep separate lists of watches and timeouts

DBusLoop-keep-separate-lists-of-watches-and-timeou.patch (text/plain), 10.67 KB, created by Simon McVittie on 2011-01-24 07:51:27 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Simon McVittie

Created: 2011-01-24 07:51:27 UTC

Size: 10.67 KB

patch

obsolete

>From 97b152938ed469bd6b12776ff72519f598d36ea4 Mon Sep 17 00:00:00 2001
>From: Simon McVittie <simon.mcvittie@collabora.co.uk>
>Date: Fri, 21 Jan 2011 18:37:26 +0000
>Subject: [PATCH] DBusLoop: keep separate lists of watches and timeouts
>
>Bug: https://bugs.freedesktop.org/show_bug.cgi?id=33342
>---
> dbus/dbus-mainloop.c |  166 ++++++++++++++++++++++++++-----------------------
> 1 files changed, 88 insertions(+), 78 deletions(-)
>
>diff --git a/dbus/dbus-mainloop.c b/dbus/dbus-mainloop.c
>index c315e66..e96010d 100644
>--- a/dbus/dbus-mainloop.c
>+++ b/dbus/dbus-mainloop.c
>@@ -56,7 +56,8 @@ watch_flags_to_string (int flags)
> struct DBusLoop
> {
>   int refcount;
>-  DBusList *callbacks;
>+  DBusList *watches;
>+  DBusList *timeouts;
>   int callback_list_serial;
>   int watch_count;
>   int timeout_count;
>@@ -185,9 +186,11 @@ callback_unref (Callback *cb)
> 
> static dbus_bool_t
> add_callback (DBusLoop  *loop,
>+              DBusList **list,
>               Callback *cb)
> {
>-  if (!_dbus_list_append (&loop->callbacks, cb))
>+
>+  if (!_dbus_list_append (list, cb))
>     return FALSE;
> 
>   loop->callback_list_serial += 1;
>@@ -207,6 +210,7 @@ add_callback (DBusLoop  *loop,
> 
> static void
> remove_callback (DBusLoop  *loop,
>+                 DBusList **list,
>                  DBusList *link)
> {
>   Callback *cb = link->data;
>@@ -222,7 +226,7 @@ remove_callback (DBusLoop  *loop,
>     }
>   
>   callback_unref (cb);
>-  _dbus_list_remove_link (&loop->callbacks, link);
>+  _dbus_list_remove_link (list, link);
>   loop->callback_list_serial += 1;
> }
> 
>@@ -281,7 +285,7 @@ _dbus_loop_add_watch (DBusLoop  *loop,
>   if (wcb == NULL)
>     return FALSE;
> 
>-  if (!add_callback (loop, (Callback*) wcb))
>+  if (!add_callback (loop, &loop->watches, (Callback*) wcb))
>     {
>       callback_unref ((Callback*) wcb);
>       return FALSE;
>@@ -300,17 +304,18 @@ _dbus_loop_remove_watch (DBusLoop         *loop,
>    * them */
>   _dbus_assert (dbus_watch_get_socket (watch) != -1);
> 
>-  link = _dbus_list_get_first_link (&loop->callbacks);
>+  link = _dbus_list_get_first_link (&loop->watches);
>   while (link != NULL)
>     {
>-      DBusList *next = _dbus_list_get_next_link (&loop->callbacks, link);
>-      Callback *this = link->data;
>+      DBusList *next = _dbus_list_get_next_link (&loop->watches, link);
>+      WatchCallback *this = link->data;
>+
>+      _dbus_assert (this->callback.type == CALLBACK_WATCH);
> 
>-      if (this->type == CALLBACK_WATCH &&
>-          WATCH_CALLBACK (this)->watch == watch)
>+      if (this->watch == watch)
>         {
>-          remove_callback (loop, link);
>-          
>+          remove_callback (loop, &loop->watches, link);
>+
>           return;
>         }
>       
>@@ -330,7 +335,7 @@ _dbus_loop_add_timeout (DBusLoop           *loop,
>   if (tcb == NULL)
>     return FALSE;
> 
>-  if (!add_callback (loop, (Callback*) tcb))
>+  if (!add_callback (loop, &loop->timeouts, (Callback*) tcb))
>     {
>       callback_unref ((Callback*) tcb);
>       return FALSE;
>@@ -345,16 +350,17 @@ _dbus_loop_remove_timeout (DBusLoop           *loop,
> {
>   DBusList *link;
>   
>-  link = _dbus_list_get_first_link (&loop->callbacks);
>+  link = _dbus_list_get_first_link (&loop->timeouts);
>   while (link != NULL)
>     {
>-      DBusList *next = _dbus_list_get_next_link (&loop->callbacks, link);
>-      Callback *this = link->data;
>+      DBusList *next = _dbus_list_get_next_link (&loop->timeouts, link);
>+      TimeoutCallback *this = link->data;
>+
>+      _dbus_assert (this->callback.type == CALLBACK_TIMEOUT);
> 
>-      if (this->type == CALLBACK_TIMEOUT &&
>-          TIMEOUT_CALLBACK (this)->timeout == timeout)
>+      if (this->timeout == timeout)
>         {
>-          remove_callback (loop, link);
>+          remove_callback (loop, &loop->timeouts, link);
>           
>           return;
>         }
>@@ -545,7 +551,7 @@ _dbus_loop_iterate (DBusLoop     *loop,
>                  block, loop->depth, loop->timeout_count, loop->watch_count);
> #endif
>   
>-  if (loop->callbacks == NULL)
>+  if (loop->watches == NULL && loop->timeouts == NULL)
>     goto next_iteration;
> 
>   if (loop->watch_count > N_STACK_DESCRIPTORS)
>@@ -573,69 +579,71 @@ _dbus_loop_iterate (DBusLoop     *loop,
> 
>   /* fill our array of fds and watches */
>   n_fds = 0;
>-  link = _dbus_list_get_first_link (&loop->callbacks);
>+  link = _dbus_list_get_first_link (&loop->watches);
>   while (link != NULL)
>     {
>-      DBusList *next = _dbus_list_get_next_link (&loop->callbacks, link);
>+      DBusList *next = _dbus_list_get_next_link (&loop->watches, link);
>       Callback *cb = link->data;
>-      if (cb->type == CALLBACK_WATCH)
>+      unsigned int flags;
>+      WatchCallback *wcb;
>+      int fd;
>+
>+      _dbus_assert (cb->type == CALLBACK_WATCH);
>+
>+      wcb = WATCH_CALLBACK (cb);
>+      fd = dbus_watch_get_socket (wcb->watch);
>+
>+      if (wcb->last_iteration_oom)
>         {
>-          unsigned int flags;
>-          WatchCallback *wcb = WATCH_CALLBACK (cb);
>-          int fd = dbus_watch_get_socket (wcb->watch);
>+          /* we skip this one this time, but reenable it next time,
>+           * and have a timeout on this iteration
>+           */
>+          wcb->last_iteration_oom = FALSE;
>+          oom_watch_pending = TRUE;
> 
>-          if (wcb->last_iteration_oom)
>-            {
>-              /* we skip this one this time, but reenable it next time,
>-               * and have a timeout on this iteration
>-               */
>-              wcb->last_iteration_oom = FALSE;
>-              oom_watch_pending = TRUE;
>-              
>-              retval = TRUE; /* return TRUE here to keep the loop going,
>-                              * since we don't know the watch is inactive
>-                              */
>+          retval = TRUE; /* return TRUE here to keep the loop going,
>+                          * since we don't know the watch is inactive
>+                          */
> 
> #if MAINLOOP_SPEW
>-              _dbus_verbose ("  skipping watch on fd %d as it was out of memory last time\n",
>-                             fd);
>+          _dbus_verbose ("  skipping watch on fd %d as it was out of memory last time\n",
>+                         fd);
> #endif
>-            }
>-          else if (_DBUS_UNLIKELY (fd == -1))
>-            {
>-              _dbus_warn ("watch %p was invalidated but not removed; "
>-                  "removing it now\n", wcb->watch);
>-              _dbus_loop_remove_watch (loop, wcb->watch);
>-            }
>-          else if (dbus_watch_get_enabled (wcb->watch))
>-            {
>-              watches_for_fds[n_fds] = wcb;
>+        }
>+      else if (_DBUS_UNLIKELY (fd == -1))
>+        {
>+          _dbus_warn ("watch %p was invalidated but not removed; "
>+              "removing it now\n", wcb->watch);
>+          _dbus_loop_remove_watch (loop, wcb->watch);
>+        }
>+      else if (dbus_watch_get_enabled (wcb->watch))
>+        {
>+          watches_for_fds[n_fds] = wcb;
> 
>-              callback_ref (cb);
>-                  
>-              flags = dbus_watch_get_flags (wcb->watch);
>-                  
>-              fds[n_fds].fd = fd;
>-              fds[n_fds].revents = 0;
>-              fds[n_fds].events = watch_flags_to_poll_events (flags);
>+          callback_ref (cb);
>+
>+          flags = dbus_watch_get_flags (wcb->watch);
>+
>+          fds[n_fds].fd = fd;
>+          fds[n_fds].revents = 0;
>+          fds[n_fds].events = watch_flags_to_poll_events (flags);
> 
> #if MAINLOOP_SPEW
>-              _dbus_verbose ("  polling watch on fd %d  %s\n",
>-                             fd, watch_flags_to_string (flags));
>+          _dbus_verbose ("  polling watch on fd %d  %s\n",
>+                         loop->fds[loop->n_fds].fd, watch_flags_to_string (flags));
> #endif
> 
>-              n_fds += 1;
>-            }
>-          else
>-            {
>+          n_fds += 1;
>+        }
>+      else
>+        {
> #if MAINLOOP_SPEW
>-              _dbus_verbose ("  skipping disabled watch on fd %d  %s\n",
>-                             fd,
>-                             watch_flags_to_string (dbus_watch_get_flags (wcb->watch)));
>+          _dbus_verbose ("  skipping disabled watch on fd %d  %s\n",
>+                         fd,
>+                         watch_flags_to_string (dbus_watch_get_flags (wcb->watch)));
> #endif
>-            }
>         }
>-              
>+
>       link = next;
>     }
>   
>@@ -646,15 +654,16 @@ _dbus_loop_iterate (DBusLoop     *loop,
>       unsigned long tv_usec;
>       
>       _dbus_get_current_time (&tv_sec, &tv_usec);
>-          
>-      link = _dbus_list_get_first_link (&loop->callbacks);
>+
>+      link = _dbus_list_get_first_link (&loop->timeouts);
>       while (link != NULL)
>         {
>-          DBusList *next = _dbus_list_get_next_link (&loop->callbacks, link);
>+          DBusList *next = _dbus_list_get_next_link (&loop->timeouts, link);
>           Callback *cb = link->data;
> 
>-          if (cb->type == CALLBACK_TIMEOUT &&
>-              dbus_timeout_get_enabled (TIMEOUT_CALLBACK (cb)->timeout))
>+          _dbus_assert (cb->type == CALLBACK_TIMEOUT);
>+
>+          if (dbus_timeout_get_enabled (TIMEOUT_CALLBACK (cb)->timeout))
>             {
>               TimeoutCallback *tcb = TIMEOUT_CALLBACK (cb);
>               int msecs_remaining;
>@@ -677,7 +686,7 @@ _dbus_loop_iterate (DBusLoop     *loop,
>                 break; /* it's not going to get shorter... */
>             }
> #if MAINLOOP_SPEW
>-          else if (cb->type == CALLBACK_TIMEOUT)
>+          else
>             {
>               _dbus_verbose ("  skipping disabled timeout\n");
>             }
>@@ -718,10 +727,10 @@ _dbus_loop_iterate (DBusLoop     *loop,
>       _dbus_get_current_time (&tv_sec, &tv_usec);
> 
>       /* It'd be nice to avoid this O(n) thingy here */
>-      link = _dbus_list_get_first_link (&loop->callbacks);
>+      link = _dbus_list_get_first_link (&loop->timeouts);
>       while (link != NULL)
>         {
>-          DBusList *next = _dbus_list_get_next_link (&loop->callbacks, link);
>+          DBusList *next = _dbus_list_get_next_link (&loop->timeouts, link);
>           Callback *cb = link->data;
> 
>           if (initial_serial != loop->callback_list_serial)
>@@ -729,9 +738,10 @@ _dbus_loop_iterate (DBusLoop     *loop,
> 
>           if (loop->depth != orig_depth)
>             goto next_iteration;
>-              
>-          if (cb->type == CALLBACK_TIMEOUT &&
>-              dbus_timeout_get_enabled (TIMEOUT_CALLBACK (cb)->timeout))
>+
>+          _dbus_assert (cb->type == CALLBACK_TIMEOUT);
>+
>+          if (dbus_timeout_get_enabled (TIMEOUT_CALLBACK (cb)->timeout))
>             {
>               TimeoutCallback *tcb = TIMEOUT_CALLBACK (cb);
>               int msecs_remaining;
>@@ -762,7 +772,7 @@ _dbus_loop_iterate (DBusLoop     *loop,
>                 }
>             }
> #if MAINLOOP_SPEW
>-          else if (cb->type == CALLBACK_TIMEOUT)
>+          else
>             {
>               _dbus_verbose ("  skipping invocation of disabled timeout\n");
>             }
>-- 
>1.7.2.3

Actions: View | Splinter Review

Attachments on bug 33342: 42374 | 42375 | 42376 | 42377 | 42378 | 42379 | 42380 | 42381 | 42382 | 42383 | 42384 | 42450 | 42451 | 42452 | 42453

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.