[PATCH] Iteration wakeup
Fan Wu
wufan9418 at gmail.com
Tue Aug 7 18:05:25 PDT 2007
Hi,
Following is the Windows patch for the fix of iteration wakeup. The
implementation of the Windows fix has been described in the first post
of the thread. The fix leverages the fact that some Windows wait
functions are alertable by queuing an APC (asynchronous procedure
call) to the thread's APC queue. The current windbus code has an
implementation of _dbus_poll which is built on top of the alertable
WSAWaitForMultipleEvents(). But the problem with
WSAWaitForMultipleEvents() is Windows does not deliver FD_WRITE (write
ready) events to the caller unless the previous write failed with
EWOULDBLOCK. The first solution I had for the problem is to use the
DBusSocket (a wrapper of Windows socket handles) to track the previous
write status. It was working but when I checked the latest windbus
code only to find DBusSocket has been removed. The removal is
understandable as the code becomes more clear, but I have to think of
another solution. With no data structure at hand to track the previous
write status, the alternative approach I can think of is to use both
poll/select and WSAWaitForMultipleEvents():
- if the timeout is short (less than 1 millisecond), then just use poll/select
- if write events are polled, do a quick check with poll/select, and
go on to WSAWaitForMultipleEvents if not sockets are ready
- if no write events are polled then just use WSAWaitForMultipleEvents()
Following is the code specifically for the poll fix. Attached file
also contains the fix for iteration wakeup which is the same as in the
previous post.
Please review. Thanks,
Fan
Index: cmake/dbus/dbus-1.def.cmake
===================================================================
--- cmake/dbus/dbus-1.def.cmake (revision 743)
+++ cmake/dbus/dbus-1.def.cmake (working copy)
@@ -728,6 +728,7 @@
dbus_connection_steal_borrowed_message
dbus_connection_unref
dbus_connection_unregister_object_path
+dbus_connection_wakeup_iteration
dbus_error_free
dbus_error_has_name
dbus_error_init
Index: dbus/dbus-sysdeps-win.c
===================================================================
--- dbus/dbus-sysdeps-win.c (revision 743)
+++ dbus/dbus-sysdeps-win.c (working copy)
@@ -1104,20 +1104,12 @@
return FALSE;
}
-/**
- * Wrapper for poll().
- *
- * @param fds the file descriptors to poll
- * @param n_fds number of descriptors in the array
- * @param timeout_milliseconds timeout or -1 for infinite
- * @returns numbers of fds with revents, or <0 on error
- */
-#define USE_CHRIS_IMPL 0
-#if USE_CHRIS_IMPL
+
+
int
-_dbus_poll (DBusPollFD *fds,
- int n_fds,
- int timeout_milliseconds)
+_dbus_poll_wsawait (DBusPollFD *fds,
+ int n_fds,
+ int timeout_milliseconds)
{
#define DBUS_POLL_CHAR_BUFFER_SIZE 2000
char msg[DBUS_POLL_CHAR_BUFFER_SIZE];
@@ -1185,15 +1177,23 @@
}
- ready = WSAWaitForMultipleEvents (n_fds, pEvents, FALSE,
timeout_milliseconds, FALSE);
+ ready = WSAWaitForMultipleEvents (n_fds, pEvents, FALSE,
timeout_milliseconds, TRUE);
+
+ for (i = 0; i < n_fds; i++)
+ fds[i].revents = 0;
- if (DBUS_SOCKET_API_RETURNS_ERROR (ready))
+ if ( WSA_WAIT_FAILED == ready )
{
DBUS_SOCKET_SET_ERRNO ();
if (errno != EWOULDBLOCK)
_dbus_verbose ("WSAWaitForMultipleEvents: failed: %s\n",
_dbus_strerror (errno));
ret = -1;
}
+ else if (WSA_WAIT_IO_COMPLETION == ready)
+ {
+ _dbus_verbose ("WSAWaitForMultipleEvents:
WSA_WAIT_IO_COMPLETION (interrupted)\n");
+ ret = -1;
+ }
else if (ready == WSA_WAIT_TIMEOUT)
{
_dbus_verbose ("WSAWaitForMultipleEvents: WSA_WAIT_TIMEOUT\n");
@@ -1209,8 +1209,6 @@
DBusPollFD *fdp = &fds[i];
WSANETWORKEVENTS ne;
- fdp->revents = 0;
-
WSAEnumNetworkEvents(fdp->fd, pEvents[i], &ne);
if (ne.lNetworkEvents & (FD_READ | FD_ACCEPT | FD_CLOSE))
@@ -1259,12 +1257,11 @@
return ret;
}
-#else // USE_CHRIS_IMPL
int
-_dbus_poll (DBusPollFD *fds,
- int n_fds,
- int timeout_milliseconds)
+_dbus_poll_regular (DBusPollFD *fds,
+ int n_fds,
+ int timeout_milliseconds)
{
#define DBUS_POLL_CHAR_BUFFER_SIZE 2000
char msg[DBUS_POLL_CHAR_BUFFER_SIZE];
@@ -1382,11 +1379,157 @@
return ready;
}
-#endif // USE_CHRIS_IMPL
+/**
+ * Wrapper for poll().
+ *
+ * The problem with WaitForMultipleEvents is FD_WRITE is only delivered if the
+ * buffer was full (EWOULDBLOCK) in the last send. This is not a problem with
+ * poll/select. So the solution is to use poll/select if timeout is short. Also
+ * if FD_WRITE events are expected, we use poll/select to do a quick
test before
+ * resorting to the WSAWait version of the poll.
+ *
+ * @param fds the file descriptors to poll
+ * @param n_fds number of descriptors in the array
+ * @param timeout_milliseconds timeout or -1 for infinite
+ * @returns numbers of fds with revents, or <0 on error
+ */
+int
+_dbus_poll (DBusPollFD *fds,
+ int n_fds,
+ int timeout_milliseconds)
+{
+ int i;
+ dbus_bool_t has_write = FALSE;
+ int t = timeout_milliseconds;
+ if ( t <= 1 && t >= 0 )
+ return _dbus_poll_regular (fds, n_fds, timeout_milliseconds );
+ for (i = 0; i < n_fds; i++)
+ if (fds[i].events & _DBUS_POLLOUT)
+ {
+ has_write = TRUE;
+ break;
+ }
+
+ if (has_write )
+ {
+ if( i = _dbus_poll_regular (fds, n_fds, 1 ))
+ return i;
+ else if ( t != -1)
+ t -= 1;
+ }
+
+ return _dbus_poll_wsawait (fds, n_fds, t );
+}
+
+/*
+ * Iteration wakeup functions. These functions shall be called with
connection locked.
+ * For Windows we use APC (asychronous procedure call) to wakeup the
thread doing the poll.
+ * In Windows every thread has an APC queue, and the thread will be
waken up from blocking if
+ * an APC is queued. A prerequisite is that the blocking must be
alertable (the alertable
+ * parameter set to TRUE when the blocking API is called).
+ *
+ */
+
+VOID
+CALLBACK APCProc(ULONG_PTR dwParam)
+{
+ /*we use this empty APC callback to wake up the WSAWait call (which
has to be alertable)*/
+}
+
+
+typedef struct
+{
+ int fd;
+ DWORD thread_id; /*the handle of the polling thread*/
+
+}DBusIterationWakeupData;
+
+
+dbus_bool_t
+_dbus_iteration_wakeup_initialize (void* data)
+{
+ DBusIterationWakeupData* p =
(DBusIterationWakeupData*)malloc(sizeof(DBusIterationWakeupData));
+
+ if (NULL == p) return FALSE;
+
+ p->fd = 0; /*we use APC, but still have this field so as to be
consistent with unix*/
+
+ p->thread_id = 0;
+
+ *(DBusIterationWakeupData**)data = p;
+ return TRUE;
+}
+
+/*
+ * */
+dbus_bool_t
+_dbus_iteration_wakeup (void* data)
+{
+ DWORD result;
+ dbus_bool_t ret = FALSE;
+ DBusIterationWakeupData* p = (DBusIterationWakeupData*)data;
+
+ if (NULL == p )
+ return ret;
+
+ if( p->thread_id)
+ {
+ HANDLE h = OpenThread( THREAD_ALL_ACCESS, FALSE, p->thread_id);
+
+ if (NULL == h) return ret;
+
+ if (QueueUserAPC(APCProc, h, (ULONG_PTR)NULL))
+ {
+ _dbus_verbose (" tried to wakeup iteration \n");
+ CloseHandle(h);
+ ret = TRUE;
+ }
+ }
+
+ return ret;
+}
+
+
+void
+_dbus_iteration_wakeup_free (void* data)
+{
+ DBusIterationWakeupData* p = *(DBusIterationWakeupData**)data;
+
+ if (NULL == p) return;
+
+ free(p);
+ *(DBusIterationWakeupData**)data = NULL;
+}
+
+
+void
+_dbus_iteration_wakeup_reset (void* data)
+{
+
+}
+
+
+void
+_dbus_iteration_wakeup_update (void* data, dbus_bool_t polling_now)
+{
+ DBusIterationWakeupData* p = (DBusIterationWakeupData*)data;
+
+ if (NULL == p) return;
+
+ if (polling_now)
+ p->thread_id = GetCurrentThreadId();
+ else
+ p->thread_id = 0;
+
+}
+/*
+ * End definiation of iteration wakeup function.
+ * */
+
/******************************************************************************
Original CVS version of
-------------- next part --------------
A non-text attachment was scrubbed...
Name: iteration_wakeup_win.patch
Type: application/octet-stream
Size: 17854 bytes
Desc: not available
Url : http://lists.freedesktop.org/archives/dbus/attachments/20070807/a9bf12ca/attachment-0001.obj
More information about the dbus
mailing list