From: Michael Tremer <michael.tremer@ipfire.org>
Date: Wed, 17 Aug 2022 09:16:31 +0000 (+0000)
Subject: jail: Make waiting loop more efficient
X-Git-Tag: 0.9.28~480
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=e068b964df3b56e55d35068ca0aaebff23326319;p=pakfire.git

jail: Make waiting loop more efficient

When the log file descriptors were closed, epoll() was returning an
event which has not been handled. That caused the loop to use 100% CPU.

Signed-off-by: Michael Tremer <michael.tremer@ipfire.org>
---

diff --git a/src/libpakfire/jail.c b/src/libpakfire/jail.c
index 15caedf09..c4b19833f 100644
--- a/src/libpakfire/jail.c
+++ b/src/libpakfire/jail.c
@@ -596,7 +596,7 @@ static int pakfire_jail_wait(struct pakfire_jail* jail, struct pakfire_jail_exec
 		goto ERROR;
 	}
 
-	ev.events = EPOLLIN;
+	ev.events = EPOLLIN|EPOLLHUP;
 
 	// Turn file descriptors into non-blocking mode and add them to epoll()
 	for (unsigned int i = 0; i < sizeof(fds) / sizeof(*fds); i++) {
@@ -632,6 +632,7 @@ static int pakfire_jail_wait(struct pakfire_jail* jail, struct pakfire_jail_exec
 		}
 
 		for (int i = 0; i < num; i++) {
+			int e  = events[i].events;
 			int fd = events[i].data.fd;
 
 			struct pakfire_log_buffer* buffer = NULL;
@@ -639,63 +640,76 @@ static int pakfire_jail_wait(struct pakfire_jail* jail, struct pakfire_jail_exec
 			void* data = NULL;
 			int priority;
 
-			// Handle any changes to the PIDFD
-			if (fd == pidfd) {
-				// Call waidid() and store the result
-				r = waitid(P_PIDFD, ctx->pidfd, &ctx->status, WEXITED);
-				if (r) {
-					ERROR(jail->pakfire, "waitid() failed: %m\n");
-					goto ERROR;
-				}
+			// Check if there is any data to be read
+			if (e & EPOLLIN) {
+				// Handle any changes to the PIDFD
+				if (fd == pidfd) {
+					// Call waidid() and store the result
+					r = waitid(P_PIDFD, ctx->pidfd, &ctx->status, WEXITED);
+					if (r) {
+						ERROR(jail->pakfire, "waitid() failed: %m\n");
+						goto ERROR;
+					}
 
-				// Mark that we have ended so that we will process the remaining
-				// events from epoll() now, but won't restart the outer loop.
-				ended = 1;
-				continue;
+					// Mark that we have ended so that we will process the remaining
+					// events from epoll() now, but won't restart the outer loop.
+					ended = 1;
+					continue;
 
-			// Handle logging messages
-			} else if (fd == log_INFO) {
-				buffer = &ctx->buffers.log_INFO;
-				priority = LOG_INFO;
+				// Handle logging messages
+				} else if (fd == log_INFO) {
+					buffer = &ctx->buffers.log_INFO;
+					priority = LOG_INFO;
 
-				callback = pakfire_jail_default_log_callback;
+					callback = pakfire_jail_default_log_callback;
 
-			} else if (fd == log_ERROR) {
-				buffer = &ctx->buffers.log_ERROR;
-				priority = LOG_ERR;
+				} else if (fd == log_ERROR) {
+					buffer = &ctx->buffers.log_ERROR;
+					priority = LOG_ERR;
 
-				callback = pakfire_jail_default_log_callback;
+					callback = pakfire_jail_default_log_callback;
 
-			} else if (fd == log_DEBUG) {
-				buffer = &ctx->buffers.log_DEBUG;
-				priority = LOG_DEBUG;
+				} else if (fd == log_DEBUG) {
+					buffer = &ctx->buffers.log_DEBUG;
+					priority = LOG_DEBUG;
 
-				callback = pakfire_jail_default_log_callback;
+					callback = pakfire_jail_default_log_callback;
 
-			// Handle anything from the log pipes
-			} else if (fd == stdout) {
-				buffer = &ctx->buffers.stdout;
-				priority = LOG_INFO;
+				// Handle anything from the log pipes
+				} else if (fd == stdout) {
+					buffer = &ctx->buffers.stdout;
+					priority = LOG_INFO;
 
-				callback = jail->log_callback;
-				data = jail->log_data;
+					callback = jail->log_callback;
+					data = jail->log_data;
 
-			} else if (fd == stderr) {
-				buffer = &ctx->buffers.stderr;
-				priority = LOG_ERR;
+				} else if (fd == stderr) {
+					buffer = &ctx->buffers.stderr;
+					priority = LOG_ERR;
 
-				callback = jail->log_callback;
-				data = jail->log_data;
+					callback = jail->log_callback;
+					data = jail->log_data;
 
-			} else {
-				DEBUG(jail->pakfire, "Received invalid file descriptor %d\n", fd);
-				continue;
+				} else {
+					DEBUG(jail->pakfire, "Received invalid file descriptor %d\n", fd);
+					continue;
+				}
+
+				// Handle log event
+				r = pakfire_jail_handle_log(jail, ctx, priority, fd, buffer, callback, data);
+				if (r)
+					goto ERROR;
 			}
 
-			// Handle log event
-			r = pakfire_jail_handle_log(jail, ctx, priority, fd, buffer, callback, data);
-			if (r)
-				goto ERROR;
+			// Check if any file descriptors have been closed
+			if (e & EPOLLHUP) {
+				// Remove the file descriptor
+				r = epoll_ctl(epollfd, EPOLL_CTL_DEL, fd, NULL);
+				if (r) {
+					ERROR(jail->pakfire, "Could not remove closed file-descriptor %d: %m\n", fd);
+					goto ERROR;
+				}
+			}
 		}
 	}