Hello Justin,
I think your fix is not correct. In some of my applications I crossed
exactly this situation and my main event loop took 100% CPU time because
of this returned error. I use an event loop like the following one
(running is modified by another thread which causes the event loop to
terminate):
volatile apr_int32_t running = 1;
void my_event_loop()
{
apr_interval_time_t timeout = 1000000; /* one second
const apr_pollfd_t *desc;
apr_int32_t num=0;
apr_status_t rc;
do {
rc = apr_pollset_poll(mypollset, timeout, &num, &desc);
if(APR_STATUS_IS_EINVAL(rc)) {
apr_sleep(timeout);
num=0;
rc = APR_TIMEUP;
}
} while(running);
}
Note the if-statement with APR_STATUS_IS_EINVAL() check. I think this
has to go into the APR source apr_pollset_poll() for Windows. Otherwise
this eventloop will do a busy looping and consume 100% CPU time of the
current thread!
Also I think the EINVAL return code should be returned if the timeout
value is INFINITE!?
Any comments?
Regards,
Stefan
Justin Erenkrantz wrote:
The patch below fixes a cosmetic problem seen with serf on Win32 if
apr_pollset_poll is called without any sockets already in the pollset.
select on Win32 will immediately return WSAEINVAL (730022) in this
case.
The MSDN docs for select() -
http://msdn.microsoft.com/en-us/library/ms740141.aspx - says:
---
Any two of the parameters, readfds, writefds, or exceptfds, can be
given as null. At least one must be non-null, and any non-null
descriptor set must contain at least one handle to a socket.
---
Earlier in the doc, it says that select() ignores the first parameter
(nfds), so it looks like while other OSes may have a short-circuit
success on the 0 nfds case, Win32 will just return WSAEINVAL in this
case. Instead of returning an error, I believe the right thing is for
APR to hide this and return success.
So, any objections to committing the following? (If we're going to do
1.3.5, I'd like to see this backported too.)
Thanks! -- justin
Index: poll/unix/select.c
===================================================================
--- poll/unix/select.c (revision 689358)
+++ poll/unix/select.c (working copy)
@@ -453,6 +453,17 @@ APR_DECLARE(apr_status_t) apr_pollset_poll(apr_pol
fd_set readset, writeset, exceptset;
apr_status_t rv = APR_SUCCESS;
+#ifdef WIN32
+ /* On Win32, select() must be presented with at least one socket to
+ * poll on, or select() will return WSAEINVAL. So, we'll just
+ * short-circuit and bail now.
+ */
+ if (pollset->nelts == 0) {
+ (*num) = 0;
+ return APR_SUCCESS;
+ }
+#endif
+
if (timeout < 0) {
tvptr = NULL;
}
