Attachment 6510 Details for Bug 8197 – Possible better patch for master (needs testing)

[patch] Possible better patch for master (needs testing)

tmp.diff (text/plain), 4.38 KB, created by Stefan Metzmacher on 2011-06-01 11:31:54 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Stefan Metzmacher

Created: 2011-06-01 11:31:54 UTC

Size: 4.38 KB

patch

obsolete

>From 2671fff66f7810e70ca336b0710d2b0d87d874f4 Mon Sep 17 00:00:00 2001
>From: Stefan Metzmacher <metze@samba.org>
>Date: Wed, 1 Jun 2011 13:08:11 +0200
>Subject: [PATCH 1/2] Revert "s3: Mark sockets in error state writable"
>
>This reverts commit 5f6f71956460d6840c1433b59e20555268b622ac.
>
>A better fix will follow, which matches the select behavior.
>
>metze
>---
> source3/lib/events.c |   23 -----------------------
> 1 files changed, 0 insertions(+), 23 deletions(-)
>
>diff --git a/source3/lib/events.c b/source3/lib/events.c
>index 499d92e..9ff1488 100644
>--- a/source3/lib/events.c
>+++ b/source3/lib/events.c
>@@ -260,29 +260,6 @@ bool run_events_poll(struct tevent_context *ev, int pollrtn,
> 
> 		if (pfd->revents & (POLLIN|POLLHUP|POLLERR)) {
> 			flags |= EVENT_FD_READ;
>-
>-			if ((fde->flags & EVENT_FD_READ) == 0) {
>-				/*
>-				 * This one is a bit subtle. If a socket is
>-				 * not being asked for readability and dies
>-				 * with POLLHUP|POLLERR, then the write
>-				 * handler must be activated to detect the
>-				 * dead socket with a failed write(2)
>-				 * call. The error I've seen is winbind
>-				 * spinning in poll trying to send something
>-				 * to a DC on a dead socket. poll gave
>-				 * POLLHUP|POLLERR, but because winbind at
>-				 * this moment only had asked for socket
>-				 * writability, it spun.
>-				 *
>-				 * We can't activate EVENT_FD_WRITE though
>-				 * whenever we have an error condition via
>-				 * POLLHUP|POLLERR, because at least smbd
>-				 * monitors EVENT_FD_WRITE in its callback,
>-				 * doing nothing.
>-				 */
>-				flags |= EVENT_FD_WRITE;
>-			}
> 		}
> 		if (pfd->revents & POLLOUT) {
> 			flags |= EVENT_FD_WRITE;
>-- 
>1.7.4.1
>
>
>From b4e98d48520d03fa4166656d4bb4052a70709ce1 Mon Sep 17 00:00:00 2001
>From: Stefan Metzmacher <metze@samba.org>
>Date: Wed, 1 Jun 2011 13:07:41 +0200
>Subject: [PATCH 2/2] s3:lib/events: only report socket errors to handlers waiting for TEVENT_FD_READ
>
>This matches the behavior of the select and epool backends in the main
>tevent library.
>
>metze
>---
> source3/lib/events.c |   37 ++++++++++++++++++++++++++++++++++---
> 1 files changed, 34 insertions(+), 3 deletions(-)
>
>diff --git a/source3/lib/events.c b/source3/lib/events.c
>index 9ff1488..7e57ed9 100644
>--- a/source3/lib/events.c
>+++ b/source3/lib/events.c
>@@ -51,6 +51,18 @@ static struct tevent_poll_private *tevent_get_poll_private(
> 	return state;
> }
> 
>+#define S3_POLL_ADDITIONAL_GOT_ERROR 0x01
>+
>+/*
>+ * if we already got an error and not waiting for
>+ * reading, we skip the event, this matches
>+ * the behavior of select.
>+ */
>+#define S3_POLL_FD_NEEDS_EVENT(fde) \
>+	(((fde)->flags & EVENT_FD_READ) || \
>+	 (((fde)->flags & EVENT_FD_WRITE) && \
>+	  !((fde)->additional_flags & S3_POLL_ADDITIONAL_GOT_ERROR)))
>+
> static void count_fds(struct tevent_context *ev,
> 		      int *pnum_fds, int *pmax_fd)
> {
>@@ -59,7 +71,7 @@ static void count_fds(struct tevent_context *ev,
> 	int max_fd = 0;
> 
> 	for (fde = ev->fd_events; fde != NULL; fde = fde->next) {
>-		if (fde->flags & (EVENT_FD_READ|EVENT_FD_WRITE)) {
>+		if (S3_POLL_FD_NEEDS_EVENT(fde)) {
> 			num_fds += 1;
> 			if (fde->fd > max_fd) {
> 				max_fd = fde->fd;
>@@ -131,7 +143,7 @@ bool event_add_to_poll_args(struct tevent_context *ev, TALLOC_CTX *mem_ctx,
> 	for (fde = ev->fd_events; fde; fde = fde->next) {
> 		struct pollfd *pfd;
> 
>-		if ((fde->flags & (EVENT_FD_READ|EVENT_FD_WRITE)) == 0) {
>+		if (!S3_POLL_FD_NEEDS_EVENT(fde)) {
> 			continue;
> 		}
> 
>@@ -243,6 +255,10 @@ bool run_events_poll(struct tevent_context *ev, int pollrtn,
> 		struct pollfd *pfd;
> 		uint16 flags = 0;
> 
>+		if (!S3_POLL_FD_NEEDS_EVENT(fde)) {
>+			continue;
>+		}
>+
> 		if (pollfd_idx[fde->fd] >= num_pfds) {
> 			DEBUG(1, ("internal error: pollfd_idx[fde->fd] (%d) "
> 				  ">= num_pfds (%d)\n", pollfd_idx[fde->fd],
>@@ -258,7 +274,22 @@ bool run_events_poll(struct tevent_context *ev, int pollrtn,
> 			return false;
> 		}
> 
>-		if (pfd->revents & (POLLIN|POLLHUP|POLLERR)) {
>+		if (pfd->revents & (POLLHUP|POLLERR)) {
>+			fde->additional_flags |= S3_POLL_ADDITIONAL_GOT_ERROR;
>+
>+			/*
>+			 * if we only wait for TEVENT_FD_WRITE, we should not tell the
>+			 * event handler about it, and remove the epoll_event,
>+			 * as we only report errors when waiting for read events,
>+			 * to match the select() behavior
>+			 */
>+			if ((fde->flags & EVENT_FD_READ) == 0) {
>+				continue;
>+			}
>+
>+			flags |= EVENT_FD_READ;
>+		}
>+		if (pfd->revents & POLLIN) {
> 			flags |= EVENT_FD_READ;
> 		}
> 		if (pfd->revents & POLLOUT) {
>-- 
>1.7.4.1
>

Actions: View

Attachments on bug 8197: 6510 | 6515 | 6528 | 6537 | 6538