watcher: Prevent busy wait if callback is active and other FDs have e…

…vents

Exiting the loop previously could cause watcher to busy wait (i.e.
rebuild the array and call poll() repeatedly) until the active callback
was done.

Assume watcher observes two FDs 15 and 22, which are in the list in that
order.  FD 15 is signaled and its callback gets triggered.  The array of
FDs is rebuilt and does not include 15 anymore.  Now FD 22 is ready for
reading.  However, when enumerating all registered FDs, the loop previously
was exited when reaching FD 15 and seeing that it's active.  FD 22 was
never checked and the array was immediately rebuilt and poll() called.
If the callback for 15 took longer, this was repeated over and over.

This basically reverts d16d5a2 ("watcher: Avoid queueing multiple
watcher callbacks at the same time"), whose goal is quite unclear to me.
If it really wanted to allow only a single callback for all FDs, it didn't
achieve that as any FD before an active one would get notified and if
multiple FDs are ready concurrently, they'd all get triggered too.
Skipping entries with active callback makes sense as it avoids a lookup
in the FD array and subsequent revents checks.  But why would we need to
rebuild the array if we see such an entry?  Once the callback is done,
the watcher is notified and the array rebuilt anyway (also if any other
FD was ready and jobs get queued).

src/libstrongswan/processing/watcher.c

@@ -388,7 +388,6 @@ static job_requeue_t watch(private_watcher_t *this)
 	entry_t *entry;
 	struct pollfd *pfd;
 	int count = 0, res;
-	bool rebuild = FALSE;
 #if DEBUG_LEVEL >= 2
 	char logbuf[BUF_LEN], *logpos, eventbuf[4], *eventpos;
 	int loglen;
@@ -451,7 +450,7 @@ static job_requeue_t watch(private_watcher_t *this)
 	}
 #endif
 
-	while (!rebuild)
+	while (TRUE)
 	{
 		int revents;
 		char buf[1];
@@ -493,7 +492,7 @@ static job_requeue_t watch(private_watcher_t *this)
 				}
 				this->pending = FALSE;
 				DBG2(DBG_JOB, "watcher got notification, rebuilding");
-				return JOB_REQUEUE_DIRECT;
+				break;
 			}
 
 			reset_log(logbuf, logpos, loglen);
@@ -502,8 +501,7 @@ static job_requeue_t watch(private_watcher_t *this)
 			{
 				if (entry->in_callback)
 				{
-					rebuild = TRUE;
-					break;
+					continue;
 				}
 				reset_event_log(eventbuf, eventpos);
 				revents = find_revents(pfd, count, entry->fd);
@@ -545,7 +543,7 @@ static job_requeue_t watch(private_watcher_t *this)
 					lib->processor->execute_job(lib->processor, job);
 				}
 				/* we temporarily disable a notified FD, rebuild FDSET */
-				return JOB_REQUEUE_DIRECT;
+				break;
 			}
 		}
 		else
@@ -554,7 +552,7 @@ static job_requeue_t watch(private_watcher_t *this)
 			{	/* complain only if no pending updates */
 				DBG1(DBG_JOB, "watcher poll() error: %s", strerror(errno));
 			}
-			return JOB_REQUEUE_DIRECT;
+			break;
 		}
 	}
 	return JOB_REQUEUE_DIRECT;