Attachment 11511 Details for Bug 11564 – Patch for master

[patch] Patch for master

patch (text/plain), 9.05 KB, created by Ralph Böhme on 2015-10-20 15:19:00 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Ralph Böhme

Created: 2015-10-20 15:19:00 UTC

Size: 9.05 KB

patch

obsolete

>From 7ed51f53fdfd3c06d8a7534c5533f3551dcc6d8a Mon Sep 17 00:00:00 2001
>From: Ralph Boehme <slow@samba.org>
>Date: Sun, 18 Oct 2015 22:23:20 +0200
>Subject: [PATCH 1/2] selftest: add a test for async_connect_send()
>
>Bug: https://bugzilla.samba.org/show_bug.cgi?id=11564
>
>Signed-off-by: Ralph Boehme <slow@samba.org>
>---
> lib/async_req/async_connect_send_test.c | 128 ++++++++++++++++++++++++++++++++
> lib/async_req/wscript_build             |   4 +
> source3/script/tests/test_async_req.sh  |  11 +++
> source3/selftest/tests.py               |   3 +
> 4 files changed, 146 insertions(+)
> create mode 100644 lib/async_req/async_connect_send_test.c
> create mode 100755 source3/script/tests/test_async_req.sh
>
>diff --git a/lib/async_req/async_connect_send_test.c b/lib/async_req/async_connect_send_test.c
>new file mode 100644
>index 0000000..75abaae
>--- /dev/null
>+++ b/lib/async_req/async_connect_send_test.c
>@@ -0,0 +1,128 @@
>+/*
>+ * Test async connect
>+ * Copyright (C) Ralph Boehme 2015
>+ *
>+ * This program is free software; you can redistribute it and/or modify
>+ * it under the terms of the GNU General Public License as published by
>+ * the Free Software Foundation; either version 3 of the License, or
>+ * (at your option) any later version.
>+ *
>+ * This program is distributed in the hope that it will be useful,
>+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
>+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>+ * GNU General Public License for more details.
>+ *
>+ * You should have received a copy of the GNU General Public License
>+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
>+ */
>+
>+#include "replace.h"
>+#include "lib/tevent/tevent.h"
>+#include "lib/async_req/async_sock.h"
>+#include <stdio.h>
>+#include <string.h>
>+#include <stdlib.h>
>+#include <errno.h>
>+#include <sys/types.h>
>+#include <sys/wait.h>
>+
>+int main(int argc, const char *argv[])
>+{
>+	int result, listen_sock, status;
>+	struct sockaddr_in addr = { 0 };
>+	pid_t pid;
>+	int port;
>+
>+	listen_sock = socket(PF_INET, SOCK_STREAM, 0);
>+	if (listen_sock == -1) {
>+		perror("socket() failed");
>+		exit(1);
>+	}
>+
>+	addr.sin_family = AF_INET;
>+	addr.sin_addr.s_addr = inet_addr("127.0.0.1");
>+
>+	for (port = 1024; port < INT32_MAX; port++) {
>+		addr.sin_port = htons(port);
>+		result = bind(listen_sock, (struct sockaddr *)&addr, sizeof(addr));
>+		if (result == 0) {
>+			break;
>+		}
>+	}
>+
>+	if (port == INT32_MAX) {
>+		printf("Huh, no free port?\n");
>+		return 1;
>+	}
>+
>+	result = listen(listen_sock, 1);
>+	if (result == -1) {
>+		perror("listen() failed");
>+		close(listen_sock);
>+		return 1;
>+	}
>+
>+	pid = fork();
>+	if (pid == -1) {
>+		perror("fork");
>+		return 1;
>+	}
>+
>+	if (pid == 0) {
>+		struct tevent_context *ev;
>+		struct tevent_req *req;
>+		int fd;
>+
>+		ev = tevent_context_init(NULL);
>+		if (ev == NULL) {
>+			fprintf(stderr, "tevent_context_init failed\n");
>+			return 1;
>+		}
>+
>+		fd = socket(PF_INET, SOCK_STREAM, 0);
>+		if (fd == -1) {
>+			perror("socket");
>+			return 1;
>+		}
>+
>+		memset(&addr, sizeof(addr), 0);
>+		addr.sin_family = AF_INET;
>+		addr.sin_port = htons(port);
>+		addr.sin_addr.s_addr = inet_addr("127.0.0.1");
>+
>+		req = async_connect_send(ev, ev, fd,
>+					 (struct sockaddr *)&addr,
>+					 sizeof(struct sockaddr_in),
>+					 NULL, NULL, NULL);
>+
>+		if (!tevent_req_poll(req, ev)) {
>+			perror("tevent_req_poll() failed");
>+			return 1;
>+		}
>+
>+		status = 0;
>+		result = async_connect_recv(req, &status);
>+		if (result != 0) {
>+			return status;
>+		}
>+		return 0;
>+	}
>+
>+	result = waitpid(pid, &status, 0);
>+	if (result == -1) {
>+		perror("waitpid");
>+		return 1;
>+	}
>+
>+	if (!WIFEXITED(status)) {
>+		printf("child status: %d\n", status);
>+		return 2;
>+	}
>+
>+	printf("test done: status=%d\n", WEXITSTATUS(status));
>+	if (status != 0) {
>+		return status;
>+	}
>+
>+	return 0;
>+}
>diff --git a/lib/async_req/wscript_build b/lib/async_req/wscript_build
>index e8af569..9c25223 100644
>--- a/lib/async_req/wscript_build
>+++ b/lib/async_req/wscript_build
>@@ -7,3 +7,7 @@ bld.SAMBA_SUBSYSTEM('LIBASYNC_REQ',
> 	deps='tevent-util socket-blocking'
> 	)
> 
>+bld.SAMBA_BINARY('async_connect_send_test',
>+                 source='async_connect_send_test.c',
>+                 deps='LIBASYNC_REQ'
>+)
>diff --git a/source3/script/tests/test_async_req.sh b/source3/script/tests/test_async_req.sh
>new file mode 100755
>index 0000000..a92f990
>--- /dev/null
>+++ b/source3/script/tests/test_async_req.sh
>@@ -0,0 +1,11 @@
>+#!/bin/sh
>+
>+incdir=`dirname $0`/../../../testprogs/blackbox
>+. $incdir/subunit.sh
>+
>+failed=0
>+
>+testit "async_connect_send" $VALGRIND $BINDIR/async_connect_send_test ||
>+	failed=`expr $failed + 1`
>+
>+testok $0 $failed
>diff --git a/source3/selftest/tests.py b/source3/selftest/tests.py
>index ecbb368..4f6c123 100755
>--- a/source3/selftest/tests.py
>+++ b/source3/selftest/tests.py
>@@ -249,6 +249,9 @@ plantestsuite(
>     "samba3.pthreadpool", "nt4_dc",
>     [os.path.join(samba3srcdir, "script/tests/test_pthreadpool.sh")])
> 
>+plantestsuite("samba3.async_req", "nt4_dc",
>+              [os.path.join(samba3srcdir, "script/tests/test_async_req.sh")])
>+
> #smbtorture4 tests
> 
> base = ["base.attr", "base.charset", "base.chkpath", "base.defer_open", "base.delaywrite", "base.delete",
>-- 
>2.1.0
>
>
>From 3382ca5c4445a40f70fdf4acdd51fad9bc0bc60a Mon Sep 17 00:00:00 2001
>From: Ralph Boehme <slow@samba.org>
>Date: Sun, 18 Oct 2015 22:21:10 +0200
>Subject: [PATCH 2/2] async_req: fix non-blocking connect()
>
>According to Stevens UNIX Network Programming and various other sources,
>the correct handling for non-blocking connect() is:
>
>- when the initial connect() return -1/EINPROGRESS polling the socket
>  for *writeability*
>
>- in the poll handler call getsocktopt() with SO_ERROR to get the
>  finished connect() return value
>
>Simply calling connect() a second time without error checking is
>probably wrong and not portable. For a successfull connect() Linux
>returns 0, but Solaris will return EISCONN:
>
>24254:   0.0336  0.0002 connect(4, 0xFEFFECAC, 16, SOV_DEFAULT) Err#150 EINPROGRESS
>24254:          AF_INET  name = 10.10.10.143  port = 1024
>24254:   0.0349  0.0001 port_associate(3, 4, 0x00000004, 0x0000001D,0x080648A8) = 0
>24254:   0.0495  0.0146 port_getn(3, 0xFEFFEB50, 1, 1, 0xFEFFEB60) = 1 [0]
>24254:   0.0497  0.0002 connect(4, 0x080646E4, 16, SOV_DEFAULT) Err#133 EISCONN
>24254:          AF_INET  name = 10.10.10.143  port = 1024
>
>Bug: https://bugzilla.samba.org/show_bug.cgi?id=11564
>
>Signed-off-by: Ralph Boehme <slow@samba.org>
>---
> lib/async_req/async_sock.c | 62 ++++++++++++++++++++++++----------------------
> 1 file changed, 32 insertions(+), 30 deletions(-)
>
>diff --git a/lib/async_req/async_sock.c b/lib/async_req/async_sock.c
>index bc3780c..71dd9ce 100644
>--- a/lib/async_req/async_sock.c
>+++ b/lib/async_req/async_sock.c
>@@ -118,33 +118,26 @@ struct tevent_req *async_connect_send(
> 
> 	state->result = connect(fd, address, address_len);
> 
>-	if (state->after_connect != NULL) {
>-		state->after_connect(state->private_data);
>-	}
>-
> 	if (state->result == 0) {
>+		if (state->after_connect != NULL) {
>+			state->after_connect(state->private_data);
>+		}
>+
> 		tevent_req_done(req);
> 		return tevent_req_post(req, ev);
> 	}
> 
>-	/**
>-	 * A number of error messages show that something good is progressing
>-	 * and that we have to wait for readability.
>-	 *
>-	 * If none of them are present, bail out.
>+	/*
>+	 * The only errno indicating that the connect is still in
>+	 * flight is EINPROGRESS, everything else is an error
> 	 */
> 
>-	if (!(errno == EINPROGRESS || errno == EALREADY ||
>-#ifdef EISCONN
>-	      errno == EISCONN ||
>-#endif
>-	      errno == EAGAIN || errno == EINTR)) {
>+	if (errno != EINPROGRESS) {
> 		tevent_req_error(req, errno);
> 		return tevent_req_post(req, ev);
> 	}
> 
>-	state->fde = tevent_add_fd(ev, state, fd,
>-				   TEVENT_FD_READ | TEVENT_FD_WRITE,
>+	state->fde = tevent_add_fd(ev, state, fd, TEVENT_FD_WRITE,
> 				   async_connect_connected, req);
> 	if (state->fde == NULL) {
> 		tevent_req_error(req, ENOMEM);
>@@ -189,27 +182,36 @@ static void async_connect_connected(struct tevent_context *ev,
> 	struct async_connect_state *state =
> 		tevent_req_data(req, struct async_connect_state);
> 	int ret;
>-
>-	if (state->before_connect != NULL) {
>-		state->before_connect(state->private_data);
>+	int socket_error = 0;
>+	socklen_t slen = sizeof(socket_error);
>+
>+	ret = getsockopt(state->fd, SOL_SOCKET, SO_ERROR,
>+			 &socket_error, &slen);
>+
>+	if (ret != 0) {
>+		/*
>+		 * According to Stevens this is the Solaris behaviour
>+		 * in case the connection encountered an error:
>+		 * getsockopt() fails, error is in errno
>+		 */
>+		tevent_req_error(req, errno);
>+		return;
> 	}
> 
>-	ret = connect(state->fd, (struct sockaddr *)(void *)&state->address,
>-		      state->address_len);
>+	if (socket_error != 0) {
>+		/*
>+		 * Berkeley derived implementations (including) Linux
>+		 * return the pending error via socket_error.
>+		 */
>+		tevent_req_error(req, socket_error);
>+		return;
>+	}
> 
> 	if (state->after_connect != NULL) {
> 		state->after_connect(state->private_data);
> 	}
> 
>-	if (ret == 0) {
>-		tevent_req_done(req);
>-		return;
>-	}
>-	if (errno == EINPROGRESS) {
>-		/* Try again later, leave the fde around */
>-		return;
>-	}
>-	tevent_req_error(req, errno);
>+	tevent_req_done(req);
> 	return;
> }
> 
>-- 
>2.1.0
>

Actions: View

Attachments on bug 11564: 11511 | 11512 | 11513 | 11515 | 11516 | 11521 | 11522