From 55e912d21e546d3e7bd31a7d356e608af36e7809 Mon Sep 17 00:00:00 2001 From: Suresh Sundriyal Date: Wed, 22 Apr 2015 21:34:49 -0700 Subject: [PATCH 1/4] [pretty] Don't wait indefinitely for reverse IP lookups. Reverse lookup for pretty print should be a best effort undertaking. So, + Give up on EAI_AGAIN. + Don't block on the getnameinfo call indefinitely, resolv.conf might be set up with a large timeout. + Disable lookups after the first failure. + Lots of OCD error checking. --- src/Makefile.am | 2 ++ src/pretty_printer.hh | 40 ++++++++++++++++++---- src/timer.cc | 79 +++++++++++++++++++++++++++++++++++++++++++ src/timer.hh | 33 ++++++++++++++++++ 4 files changed, 147 insertions(+), 7 deletions(-) create mode 100644 src/timer.cc create mode 100644 src/timer.hh diff --git a/src/Makefile.am b/src/Makefile.am index 2757b53a..6c3d5e3f 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -141,6 +141,7 @@ noinst_HEADERS = \ textfile_sub_source.hh \ textview_curses.hh \ time_T.hh \ + timer.hh \ top_status_source.hh \ view_curses.hh \ vt52_curses.hh \ @@ -203,6 +204,7 @@ libdiag_a_SOURCES = \ sqlite-extension-func.c \ statusview_curses.cc \ string-extension-functions.cc \ + timer.cc \ pcrepp.cc \ piper_proc.cc \ sql_util.cc \ diff --git a/src/pretty_printer.hh b/src/pretty_printer.hh index aa90cb18..e71d4ba1 100644 --- a/src/pretty_printer.hh +++ b/src/pretty_printer.hh @@ -30,18 +30,32 @@ #ifndef __pretty_printer_hh #define __pretty_printer_hh +#include #include #include #include #include +#include #include #include #include +#include "timer.hh" #include "ansi_scrubber.hh" #include "data_scanner.hh" +static sig_atomic_t reverse_lookup_enabled = 1; + +void sigalrm_handler(int sig) +{ + if (sig == SIGALRM) + { + reverse_lookup_enabled = 0; + } +} + + class pretty_printer { public: @@ -149,14 +163,26 @@ private: require(0); break; } - if (rc == 1) { - while ((rc = getnameinfo((struct sockaddr *)&sa, socklen, - buffer, sizeof(buffer), NULL, 0, - NI_NAMEREQD)) == EAI_AGAIN) { - usleep(1000); + if (rc == 1 && reverse_lookup_enabled) { + const struct timeval timeout = {3, 0}; + + { + timer::interrupt_timer t(timeout, sigalrm_handler); + if (t.arm_timer() == 0) { + rc = getnameinfo((struct sockaddr *)&sa, socklen, + buffer, sizeof(buffer), NULL, 0, + NI_NAMEREQD); + if (rc == 0) { + result = buffer; + } + } + else { + log_error("Unable to set timer, disabling reverse lookup"); + reverse_lookup_enabled = 0; + } } - if (rc == 0) { - result = buffer; + if (!reverse_lookup_enabled) { + log_info("Reverse lookup in pretty-print view disabled"); } } this->pp_stream << " " << ANSI_UNDERLINE_START << diff --git a/src/timer.cc b/src/timer.cc new file mode 100644 index 00000000..f05614dc --- /dev/null +++ b/src/timer.cc @@ -0,0 +1,79 @@ +#include "timer.hh" +#include "lnav_log.hh" + +const struct itimerval disable = { + { 0, 0 }, + { 0, 0 } +}; + +timer::error::error(int err):e_err(err) { } + +timer::interrupt_timer::interrupt_timer(struct timeval t, + sighandler_t_ sighandler=SIG_IGN) : new_handler(sighandler), + old_handler(NULL), armed(true) { + this->new_val = (struct itimerval){ + { 0, 0 }, + t + }; + this->old_val = (struct itimerval){ + { 0, 0 }, + { 0, 0 } + }; +} + +int timer::interrupt_timer::arm_timer() { + // Disable the interval timer before resetting the handler and rearming + // the previous interval timer or else we will have a race-condition + // where the timer might fire and the appropriate handler might not be + // set. + if (setitimer(ITIMER_REAL, &disable, &this->old_val) != 0) { + log_error("Unable to disable the timer: %s", + strerror(errno)); + return -1; + } + this->old_handler = signal(SIGALRM, this->new_handler); + if (this->old_handler == SIG_ERR) { + log_error("Unable to set the signal handler: %s", + strerror(errno)); + if (setitimer(ITIMER_REAL, &this->old_val, NULL) != 0) { + log_error("Unable to reset the interrupt timer: %s", strerror(errno)); + throw timer::error(errno); + } + return -1; + } + + if (setitimer(ITIMER_REAL, &this->new_val, NULL) != 0) { + if(signal(SIGALRM, this->old_handler) == SIG_ERR) { + log_error("Unable to reset the signal handler: %s", strerror(errno)); + throw timer::error(errno); + } + log_error("Unable to set the timer: %s", strerror(errno)); + return -1; + } + this->armed = true; + return 0; +} + +timer::interrupt_timer::~interrupt_timer() { + if (this->armed) { + // Disable the interval timer before resetting the handler and rearming + // the previous interval timer or else we will have a race-condition + // where the timer might fire and the appropriate handler might not be + // set. + if (setitimer(ITIMER_REAL, &disable, NULL) != 0) { + log_error("Failed to disable the timer: %s", + strerror(errno)); + throw timer::error(errno); + } + if (signal(SIGALRM, this->old_handler) == SIG_ERR) { + log_error("Failed to reinstall previous SIGALRM handler: %s", + strerror(errno)); + throw timer::error(errno); + } + if (setitimer(ITIMER_REAL, &this->old_val, NULL) != 0) { + log_error("Failed to reset the timer to previous value: %s", + strerror(errno)); + throw timer::error(errno); + } + } +} diff --git a/src/timer.hh b/src/timer.hh new file mode 100644 index 00000000..53e1ee41 --- /dev/null +++ b/src/timer.hh @@ -0,0 +1,33 @@ +#ifndef timer_hh +#define timer_hh + +#include +#include +#include +#include +#include + +// Linux and BSD seem to name the function pointer to signal handler differently. +// Linux names it 'sighandler_t' and BSD names it 'sig_t'. Instead of depending +// on configure scripts to find out the type name, typedef it right here. +typedef void (*sighandler_t_)(int); + +namespace timer{ +class error : public std::exception { + public: + error(int err); + int e_err; +}; + +class interrupt_timer { + public: + interrupt_timer(struct timeval, sighandler_t_); + int arm_timer(); + ~interrupt_timer(); + private: + struct itimerval new_val, old_val; + sighandler_t_ new_handler, old_handler; + bool armed; +}; +} +#endif From 99094a9cf8474915e65b27525276861c9c99114a Mon Sep 17 00:00:00 2001 From: Suresh Sundriyal Date: Fri, 24 Apr 2015 00:37:57 -0700 Subject: [PATCH 2/4] [timer] Some refactoring --- src/timer.cc | 25 +++++++++---------------- src/timer.hh | 2 +- 2 files changed, 10 insertions(+), 17 deletions(-) diff --git a/src/timer.cc b/src/timer.cc index f05614dc..7eb45731 100644 --- a/src/timer.cc +++ b/src/timer.cc @@ -10,22 +10,13 @@ timer::error::error(int err):e_err(err) { } timer::interrupt_timer::interrupt_timer(struct timeval t, sighandler_t_ sighandler=SIG_IGN) : new_handler(sighandler), - old_handler(NULL), armed(true) { - this->new_val = (struct itimerval){ - { 0, 0 }, - t - }; - this->old_val = (struct itimerval){ - { 0, 0 }, - { 0, 0 } - }; -} + old_handler(NULL), new_val((struct itimerval){{0,0},t}), + old_val(disable), armed(true) { } int timer::interrupt_timer::arm_timer() { - // Disable the interval timer before resetting the handler and rearming - // the previous interval timer or else we will have a race-condition - // where the timer might fire and the appropriate handler might not be - // set. + // Disable the interval timer before setting the handler and arming the + // interval timer or else we will have a race-condition where the timer + // might fire and the appropriate handler might not be set. if (setitimer(ITIMER_REAL, &disable, &this->old_val) != 0) { log_error("Unable to disable the timer: %s", strerror(errno)); @@ -36,7 +27,8 @@ int timer::interrupt_timer::arm_timer() { log_error("Unable to set the signal handler: %s", strerror(errno)); if (setitimer(ITIMER_REAL, &this->old_val, NULL) != 0) { - log_error("Unable to reset the interrupt timer: %s", strerror(errno)); + log_error("Unable to reset the interrupt timer: %s", + strerror(errno)); throw timer::error(errno); } return -1; @@ -44,7 +36,8 @@ int timer::interrupt_timer::arm_timer() { if (setitimer(ITIMER_REAL, &this->new_val, NULL) != 0) { if(signal(SIGALRM, this->old_handler) == SIG_ERR) { - log_error("Unable to reset the signal handler: %s", strerror(errno)); + log_error("Unable to reset the signal handler: %s", + strerror(errno)); throw timer::error(errno); } log_error("Unable to set the timer: %s", strerror(errno)); diff --git a/src/timer.hh b/src/timer.hh index 53e1ee41..d056c5a4 100644 --- a/src/timer.hh +++ b/src/timer.hh @@ -25,8 +25,8 @@ class interrupt_timer { int arm_timer(); ~interrupt_timer(); private: - struct itimerval new_val, old_val; sighandler_t_ new_handler, old_handler; + struct itimerval new_val, old_val; bool armed; }; } From da582ce00fd05c1f10d665a2af9967af4b409ba2 Mon Sep 17 00:00:00 2001 From: Suresh Sundriyal Date: Fri, 24 Apr 2015 00:43:30 -0700 Subject: [PATCH 3/4] [timer] Add 'disarm' method --- src/timer.cc | 6 +++++- src/timer.hh | 1 + 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/src/timer.cc b/src/timer.cc index 7eb45731..0c31f1a7 100644 --- a/src/timer.cc +++ b/src/timer.cc @@ -47,7 +47,7 @@ int timer::interrupt_timer::arm_timer() { return 0; } -timer::interrupt_timer::~interrupt_timer() { +void timer::interrupt_timer::disarm_timer() { if (this->armed) { // Disable the interval timer before resetting the handler and rearming // the previous interval timer or else we will have a race-condition @@ -70,3 +70,7 @@ timer::interrupt_timer::~interrupt_timer() { } } } + +timer::interrupt_timer::~interrupt_timer() { + this->disarm_timer(); +} diff --git a/src/timer.hh b/src/timer.hh index d056c5a4..d8f2d2d3 100644 --- a/src/timer.hh +++ b/src/timer.hh @@ -23,6 +23,7 @@ class interrupt_timer { public: interrupt_timer(struct timeval, sighandler_t_); int arm_timer(); + void disarm_timer(); ~interrupt_timer(); private: sighandler_t_ new_handler, old_handler; From fcb2db38b03c9dd3e9d702e3c7557e23d592f955 Mon Sep 17 00:00:00 2001 From: Suresh Sundriyal Date: Fri, 24 Apr 2015 01:37:58 -0700 Subject: [PATCH 4/4] [timer] Start off disarmed + refactoring. --- src/timer.cc | 38 +++++++++++++++++++++++++++++++++++++- src/timer.hh | 30 ++++++++++++++++++++++++++++++ 2 files changed, 67 insertions(+), 1 deletion(-) diff --git a/src/timer.cc b/src/timer.cc index 0c31f1a7..5dc3eb68 100644 --- a/src/timer.cc +++ b/src/timer.cc @@ -1,3 +1,32 @@ +/** + * Copyright (c) 2015, Suresh Sundriyal + * + * All rights reserved. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions are met: + * + * * Redistributions of source code must retain the above copyright notice, this + * list of conditions and the following disclaimer. + * * Redistributions in binary form must reproduce the above copyright notice, + * this list of conditions and the following disclaimer in the documentation + * and/or other materials provided with the distribution. + * * Neither the name of the copyright holder nor the names of its contributors + * may be used to endorse or promote products derived from this software + * without specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE REGENTS AND CONTRIBUTORS ''AS IS'' AND ANY + * EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED + * WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE + * DISCLAIMED. IN NO EVENT SHALL THE REGENTS OR CONTRIBUTORS BE LIABLE FOR ANY + * DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES + * (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; + * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON + * ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS + * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + */ + #include "timer.hh" #include "lnav_log.hh" @@ -11,7 +40,7 @@ timer::error::error(int err):e_err(err) { } timer::interrupt_timer::interrupt_timer(struct timeval t, sighandler_t_ sighandler=SIG_IGN) : new_handler(sighandler), old_handler(NULL), new_val((struct itimerval){{0,0},t}), - old_val(disable), armed(true) { } + old_val(disable), armed(false) { } int timer::interrupt_timer::arm_timer() { // Disable the interval timer before setting the handler and arming the @@ -47,6 +76,10 @@ int timer::interrupt_timer::arm_timer() { return 0; } +bool timer::interrupt_timer::is_armed() { + return this->armed; +} + void timer::interrupt_timer::disarm_timer() { if (this->armed) { // Disable the interval timer before resetting the handler and rearming @@ -68,6 +101,9 @@ void timer::interrupt_timer::disarm_timer() { strerror(errno)); throw timer::error(errno); } + this->armed = false; + this->old_val = disable; + this->old_handler = NULL; } } diff --git a/src/timer.hh b/src/timer.hh index d8f2d2d3..5d4e1cd2 100644 --- a/src/timer.hh +++ b/src/timer.hh @@ -1,3 +1,32 @@ +/** + * Copyright (c) 2015, Suresh Sundriyal + * + * All rights reserved. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions are met: + * + * * Redistributions of source code must retain the above copyright notice, this + * list of conditions and the following disclaimer. + * * Redistributions in binary form must reproduce the above copyright notice, + * this list of conditions and the following disclaimer in the documentation + * and/or other materials provided with the distribution. + * * Neither the name of the copyright holder nor the names of its contributors + * may be used to endorse or promote products derived from this software + * without specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE REGENTS AND CONTRIBUTORS ''AS IS'' AND ANY + * EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED + * WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE + * DISCLAIMED. IN NO EVENT SHALL THE REGENTS OR CONTRIBUTORS BE LIABLE FOR ANY + * DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES + * (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; + * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON + * ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS + * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + */ + #ifndef timer_hh #define timer_hh @@ -24,6 +53,7 @@ class interrupt_timer { interrupt_timer(struct timeval, sighandler_t_); int arm_timer(); void disarm_timer(); + bool is_armed(); ~interrupt_timer(); private: sighandler_t_ new_handler, old_handler;