From add115677940e82fd7502f4db7d59c1044339b8d Mon Sep 17 00:00:00 2001 From: Ralph Boehme 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 Also includes: selftest: Fix memset parameters in test for async_connect_send() Signed-off-by: Ralph Boehme Reviewed-by: Jeremy Allison (cherry picked from commit aa96c75346a9bad60471a206d65c7b7049b9ca83) (cherry picked from commit 7cf45539da9cba25130457941814da12d0a828c3) --- lib/async_req/async_connect_send_test.c | 130 ++++++++++++++++++++++++++++++++ lib/async_req/wscript_build | 4 + source3/script/tests/test_async_req.sh | 11 +++ source3/selftest/tests.py | 3 + 4 files changed, 148 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..34ea6b7 --- /dev/null +++ b/lib/async_req/async_connect_send_test.c @@ -0,0 +1,130 @@ +/* + * 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 . + */ + +#include "replace.h" +#include "lib/tevent/tevent.h" +#include "lib/async_req/async_sock.h" +#include +#include +#include +#include +#include +#include + +int main(int argc, const char *argv[]) +{ + int result, listen_sock, status, exit_status; + uint16_t port; + struct sockaddr_in addr = { 0 }; + pid_t pid; + + 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 < UINT16_MAX; port++) { + addr.sin_port = htons(port); + result = bind(listen_sock, (struct sockaddr *)&addr, sizeof(addr)); + if (result == 0) { + break; + } + } + + if (port == UINT16_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, 0, sizeof(addr)); + 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; + } + + exit_status = WEXITSTATUS(status); + printf("test done: status=%d\n", exit_status); + + if (exit_status != 0) { + return exit_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 2617e4c..630927b 100755 --- a/source3/selftest/tests.py +++ b/source3/selftest/tests.py @@ -248,6 +248,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.6.0.rc2.230.g3dd15c0 From 94e117bf872423c650f366aa09288b8e1cbce2a0 Mon Sep 17 00:00:00 2001 From: Ralph Boehme 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 Reviewed-by: Jeremy Allison (cherry picked from commit 05d4dbda8357712cb81008e0d611fdb0e7239587) --- lib/async_req/async_sock.c | 56 ++++++++++++++++++++++------------------------ 1 file changed, 27 insertions(+), 29 deletions(-) diff --git a/lib/async_req/async_sock.c b/lib/async_req/async_sock.c index bc3780c..c0ad8f3 100644 --- a/lib/async_req/async_sock.c +++ b/lib/async_req/async_sock.c @@ -127,24 +127,17 @@ struct tevent_req *async_connect_send( 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,32 @@ 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); - } - - ret = connect(state->fd, (struct sockaddr *)(void *)&state->address, - state->address_len); - - if (state->after_connect != NULL) { - state->after_connect(state->private_data); - } - - if (ret == 0) { - tevent_req_done(req); + 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; } - if (errno == EINPROGRESS) { - /* Try again later, leave the fde around */ + + if (socket_error != 0) { + /* + * Berkeley derived implementations (including) Linux + * return the pending error via socket_error. + */ + tevent_req_error(req, socket_error); return; } - tevent_req_error(req, errno); + + tevent_req_done(req); return; } -- 2.6.0.rc2.230.g3dd15c0