From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from localhost (localhost [127.0.0.1]) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTP id D9109250CA for ; Mon, 5 Aug 2019 09:32:44 -0400 (EDT) Received: from turing.freelists.org ([127.0.0.1]) by localhost (turing.freelists.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id ECO4oyI_pTrP for ; Mon, 5 Aug 2019 09:32:44 -0400 (EDT) Received: from smtpng2.m.smailru.net (smtpng2.m.smailru.net [94.100.179.3]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTPS id 1935C250C7 for ; Mon, 5 Aug 2019 09:32:43 -0400 (EDT) Content-Type: text/plain; charset=us-ascii Mime-Version: 1.0 (Mac OS X Mail 12.2 \(3445.102.3\)) Subject: [tarantool-patches] Re: [PATCH v2 2/2] say: take getaddrinfo() errors into account From: Roman Khabibov In-Reply-To: <20190723145249.5xwc2td6omphwwzw@tkn_work_nb> Date: Mon, 5 Aug 2019 16:32:41 +0300 Content-Transfer-Encoding: quoted-printable Message-Id: References: <3603f7507651b37ddd549a8c247709cc7ff43f44.1561469272.git.roman.habibov@tarantool.org> <20190723145249.5xwc2td6omphwwzw@tkn_work_nb> Sender: tarantool-patches-bounce@freelists.org Errors-to: tarantool-patches-bounce@freelists.org Reply-To: tarantool-patches@freelists.org List-Help: List-Unsubscribe: List-software: Ecartis version 1.0.0 List-Id: tarantool-patches List-Subscribe: List-Owner: List-post: List-Archive: To: tarantool-patches@freelists.org Cc: Alexander Turenko > On Jul 23, 2019, at 17:52, Alexander Turenko = wrote: >=20 >> @@ -594,6 +594,7 @@ log_syslog_init(struct log *log, const char = *init_str) >> log->fd =3D log_syslog_connect(log); >> if (log->fd < 0) { >> /* syslog indent is freed in atexit(). */ >> + diag_log(); >=20 > 1. It is need to be properly commented: we need to log a diagnostics > here until stacked diagnostics will be implemented (#1148). >=20 > 2. syslog_connect_unix() does not set a diag, diag_log() will lead to = an > assertion fail. >=20 > 3. I would mention this change in the commit message, because it is = not > part of the problem with Mac OS you described in it. @@ -506,10 +506,15 @@ syslog_connect_remote(const char *server_address) hints.ai_protocol =3D IPPROTO_UDP; =20 ret =3D getaddrinfo(remote, portnum, &hints, &inf); - if (ret < 0) { + if (ret !=3D 0) { errno =3D EIO; diag_set(SystemError, "getaddrinfo: %s", gai_strerror(ret)); + /* + * We need to log a diagnostics here until stacked + * diagnostics will be implemented (#1148). + */ + diag_log(); goto out; } commit f5b19e933fbf2eb3784a0c3cc28d999a3fa85abe Author: Roman Khabibov Date: Tue Jul 30 15:39:21 2019 +0300 coio/say: take getaddrinfo() errors into account =20 Before this patch, branch when getaddrinfo() returns error codes couldn't be reached on Mac OS, because they are greater than 0 on Mac OS (assumption "rc < 0" in commit ea1da04 is incorrect for Mac OS). =20 * diag log() in say.c was added, because we need to log a diagnostics here until stacked diagnostics will be implemented (#1148). =20 Need for #4138 diff --git a/src/lib/core/coio_task.c b/src/lib/core/coio_task.c index 908b336ed..83f669d05 100644 --- a/src/lib/core/coio_task.c +++ b/src/lib/core/coio_task.c @@ -413,7 +413,7 @@ coio_getaddrinfo(const char *host, const char *port, return -1; /* timed out or cancelled */ =20 /* Task finished */ - if (task->rc < 0) { + if (task->rc !=3D 0) { /* getaddrinfo() failed */ errno =3D EIO; diag_set(SystemError, "getaddrinfo: %s", diff --git a/src/lib/core/say.c b/src/lib/core/say.c index 0b2cf2c34..a45595443 100644 --- a/src/lib/core/say.c +++ b/src/lib/core/say.c @@ -506,10 +506,15 @@ syslog_connect_remote(const char *server_address) hints.ai_protocol =3D IPPROTO_UDP; =20 ret =3D getaddrinfo(remote, portnum, &hints, &inf); - if (ret < 0) { + if (ret !=3D 0) { errno =3D EIO; diag_set(SystemError, "getaddrinfo: %s", gai_strerror(ret)); + /* + * We need to log a diagnostics here until stacked + * diagnostics will be implemented (#1148). + */ + diag_log(); goto out; } for (ptr =3D inf; ptr; ptr =3D ptr->ai_next) { diff --git a/test/box-tap/cfg.test.lua b/test/box-tap/cfg.test.lua index 55de5e41c..d92e9140d 100755 --- a/test/box-tap/cfg.test.lua +++ b/test/box-tap/cfg.test.lua @@ -6,7 +6,7 @@ local socket =3D require('socket') local fio =3D require('fio') local uuid =3D require('uuid') local msgpack =3D require('msgpack') -test:plan(104) +test:plan(105) =20 = --------------------------------------------------------------------------= ------ -- Invalid values @@ -566,6 +566,23 @@ os.exit(0) ]] test:is(run_script(code), 0, "log_nonblock") =20 +-- +-- gh-4138: check getaddrinfo() error and panic after that. +-- +code=3D[[ +local socket =3D require('socket') +local log =3D require('log') +local fio =3D require('fio') + +path =3D fio.pathjoin(fio.cwd(), 'log_unix_socket_test.sock') +unix_socket =3D socket('AF_UNIX', 'SOCK_DGRAM', 0) +unix_socket:bind('unix/', path) + +opt =3D = string.format("syslog:server=3Dnon_exists_hostname:%s,identity=3Dtarantool= ", path) +box.cfg{log =3D opt, log_nonblock=3Dtrue} +]] +test:is(run_script(code), PANIC, "log_nonblock") + -- -- Crash (instead of panic) when trying to recover a huge tuple. -- diff --git a/test/unit/coio.cc b/test/unit/coio.cc index bb8bd7131..a70d3254d 100644 --- a/test/unit/coio.cc +++ b/test/unit/coio.cc @@ -72,7 +72,7 @@ static void test_getaddrinfo(void) { header(); - plan(1); + plan(3); const char *host =3D "127.0.0.1"; const char *port =3D "3333"; struct addrinfo *i; @@ -81,6 +81,12 @@ test_getaddrinfo(void) is(rc, 0, "getaddrinfo"); freeaddrinfo(i); =20 + /* gh-4138: Check getaddrinfo() error. */ + isnt(coio_getaddrinfo("non_exists_hostname", port, NULL, &i, 1), = 0, + "getaddrinfo error"); + isnt(strstr(diag_get()->last->errmsg, "getaddrinfo"), NULL, + "getaddrinfo error message"); + /* * gh-4209: 0 timeout should not be a special value and * detach a task. Before a fix it led to segfault diff --git a/test/unit/coio.result b/test/unit/coio.result index 5019fa48a..49759b747 100644 --- a/test/unit/coio.result +++ b/test/unit/coio.result @@ -7,6 +7,8 @@ # call done with res 0 *** test_call_f: done *** *** test_getaddrinfo *** -1..1 +1..3 ok 1 - getaddrinfo +ok 2 - getaddrinfo error +ok 3 - getaddrinfo error message *** test_getaddrinfo: done ***