From c7b89bbfebb473edfd4a654d87328c5ffb18f0d7 Mon Sep 17 00:00:00 2001 From: Keith James Date: Tue, 26 Jul 2022 14:58:05 +0100 Subject: [PATCH 1/3] Improve iRODS connection timeout/refresh Improve connection refresh for the case when the client it running, but blocked on STDIN, waiting for the next JSON document. Previously, any open iRODS connection would remain so until the next document could be read. By adding a separate connection timeout management thread, the connection can be closed promptly, even when blocked on reading. A mutex protects the connection from being accessed by the main and timeout management threads simultaneously. --- README | 2 + configure.ac | 19 +- doc/index.rst | 4 +- m4/ax_pthread.m4 | 522 +++++++++++++++++++++++++++++++++++++++++++++++ src/operations.c | 129 ++++++------ src/operations.h | 5 +- 6 files changed, 592 insertions(+), 89 deletions(-) create mode 100644 m4/ax_pthread.m4 diff --git a/README b/README index 71ed0124..52d0b83b 100644 --- a/README +++ b/README @@ -45,6 +45,8 @@ iRODS compatibility: 2.1.x 4.1.x - 4.2.7 3.0.x 4.2.7 - 4.2.8 3.1.x 4.2.7 - 4.2.9 + 3.2.x 4.2.7 - 4.2.11 + 3.3.x 4.2.7 - 4.2.11 Installation: diff --git a/configure.ac b/configure.ac index cb78f528..6783a9ea 100644 --- a/configure.ac +++ b/configure.ac @@ -116,24 +116,7 @@ AC_ARG_WITH([test-resource], TEST_RESOURCE="testResc"]) dnl End test resource -dnl Begin put workaround -dnl See for https://github.com/irods/irods/issues/5072 -wo_url="https://github.com/irods/irods/issues/5072" - -AC_ARG_ENABLE([put-workaround], - [AS_HELP_STRING([--enable-put-workaround], - [Enable workaround for ${wo_url} (default is no)])], - [put_workaround_enabled=${enableval}], [put_workaround_enabled=no]) - -AS_IF([test "x${put_workaround_enabled}" = "xyes"], - [AC_MSG_NOTICE([enabled workaround for ${wo_url}])] - [AC_DEFINE([ENABLE_PUT_WORKAROUND], - [], - "Workaround for ${wo_url}")], - []) -AM_CONDITIONAL(PUT_WORKAROUND_ENABLED, - [test "x${put_workaround_enabled}" = "xyes"]) -dnl End put workaround +AX_PTHREAD(, [AC_MSG_ERROR([unable to find libpthread])]) AC_CHECK_LIB([jansson], [json_unpack], [], [AC_MSG_ERROR([unable to find libjansson])]) diff --git a/doc/index.rst b/doc/index.rst index f9b2db09..cd07fc6f 100644 --- a/doc/index.rst +++ b/doc/index.rst @@ -40,6 +40,8 @@ iRODS: * Simplified API over the iRODS general query API to ease construction of new custom queries. +* Automatic release of iRODS connections when idle (see the ``--connect-time`` + command line option). .. toctree:: :maxdepth: 2 @@ -226,7 +228,7 @@ Options .. program:: baton-chmod .. option:: --unsafe - Permit relative paths, which are unsafe in iRODS 3.x - 4.1.x + Permit relative paths, which are unsafe in iRODS 3.x - 4.2.x .. program:: baton-chmod .. option:: --verbose diff --git a/m4/ax_pthread.m4 b/m4/ax_pthread.m4 new file mode 100644 index 00000000..9f35d139 --- /dev/null +++ b/m4/ax_pthread.m4 @@ -0,0 +1,522 @@ +# =========================================================================== +# https://www.gnu.org/software/autoconf-archive/ax_pthread.html +# =========================================================================== +# +# SYNOPSIS +# +# AX_PTHREAD([ACTION-IF-FOUND[, ACTION-IF-NOT-FOUND]]) +# +# DESCRIPTION +# +# This macro figures out how to build C programs using POSIX threads. It +# sets the PTHREAD_LIBS output variable to the threads library and linker +# flags, and the PTHREAD_CFLAGS output variable to any special C compiler +# flags that are needed. (The user can also force certain compiler +# flags/libs to be tested by setting these environment variables.) +# +# Also sets PTHREAD_CC and PTHREAD_CXX to any special C compiler that is +# needed for multi-threaded programs (defaults to the value of CC +# respectively CXX otherwise). (This is necessary on e.g. AIX to use the +# special cc_r/CC_r compiler alias.) +# +# NOTE: You are assumed to not only compile your program with these flags, +# but also to link with them as well. For example, you might link with +# $PTHREAD_CC $CFLAGS $PTHREAD_CFLAGS $LDFLAGS ... $PTHREAD_LIBS $LIBS +# $PTHREAD_CXX $CXXFLAGS $PTHREAD_CFLAGS $LDFLAGS ... $PTHREAD_LIBS $LIBS +# +# If you are only building threaded programs, you may wish to use these +# variables in your default LIBS, CFLAGS, and CC: +# +# LIBS="$PTHREAD_LIBS $LIBS" +# CFLAGS="$CFLAGS $PTHREAD_CFLAGS" +# CXXFLAGS="$CXXFLAGS $PTHREAD_CFLAGS" +# CC="$PTHREAD_CC" +# CXX="$PTHREAD_CXX" +# +# In addition, if the PTHREAD_CREATE_JOINABLE thread-attribute constant +# has a nonstandard name, this macro defines PTHREAD_CREATE_JOINABLE to +# that name (e.g. PTHREAD_CREATE_UNDETACHED on AIX). +# +# Also HAVE_PTHREAD_PRIO_INHERIT is defined if pthread is found and the +# PTHREAD_PRIO_INHERIT symbol is defined when compiling with +# PTHREAD_CFLAGS. +# +# ACTION-IF-FOUND is a list of shell commands to run if a threads library +# is found, and ACTION-IF-NOT-FOUND is a list of commands to run it if it +# is not found. If ACTION-IF-FOUND is not specified, the default action +# will define HAVE_PTHREAD. +# +# Please let the authors know if this macro fails on any platform, or if +# you have any other suggestions or comments. This macro was based on work +# by SGJ on autoconf scripts for FFTW (http://www.fftw.org/) (with help +# from M. Frigo), as well as ac_pthread and hb_pthread macros posted by +# Alejandro Forero Cuervo to the autoconf macro repository. We are also +# grateful for the helpful feedback of numerous users. +# +# Updated for Autoconf 2.68 by Daniel Richard G. +# +# LICENSE +# +# Copyright (c) 2008 Steven G. Johnson +# Copyright (c) 2011 Daniel Richard G. +# Copyright (c) 2019 Marc Stevens +# +# 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 . +# +# As a special exception, the respective Autoconf Macro's copyright owner +# gives unlimited permission to copy, distribute and modify the configure +# scripts that are the output of Autoconf when processing the Macro. You +# need not follow the terms of the GNU General Public License when using +# or distributing such scripts, even though portions of the text of the +# Macro appear in them. The GNU General Public License (GPL) does govern +# all other use of the material that constitutes the Autoconf Macro. +# +# This special exception to the GPL applies to versions of the Autoconf +# Macro released by the Autoconf Archive. When you make and distribute a +# modified version of the Autoconf Macro, you may extend this special +# exception to the GPL to apply to your modified version as well. + +#serial 31 + +AU_ALIAS([ACX_PTHREAD], [AX_PTHREAD]) +AC_DEFUN([AX_PTHREAD], [ +AC_REQUIRE([AC_CANONICAL_HOST]) +AC_REQUIRE([AC_PROG_CC]) +AC_REQUIRE([AC_PROG_SED]) +AC_LANG_PUSH([C]) +ax_pthread_ok=no + +# We used to check for pthread.h first, but this fails if pthread.h +# requires special compiler flags (e.g. on Tru64 or Sequent). +# It gets checked for in the link test anyway. + +# First of all, check if the user has set any of the PTHREAD_LIBS, +# etcetera environment variables, and if threads linking works using +# them: +if test "x$PTHREAD_CFLAGS$PTHREAD_LIBS" != "x"; then + ax_pthread_save_CC="$CC" + ax_pthread_save_CFLAGS="$CFLAGS" + ax_pthread_save_LIBS="$LIBS" + AS_IF([test "x$PTHREAD_CC" != "x"], [CC="$PTHREAD_CC"]) + AS_IF([test "x$PTHREAD_CXX" != "x"], [CXX="$PTHREAD_CXX"]) + CFLAGS="$CFLAGS $PTHREAD_CFLAGS" + LIBS="$PTHREAD_LIBS $LIBS" + AC_MSG_CHECKING([for pthread_join using $CC $PTHREAD_CFLAGS $PTHREAD_LIBS]) + AC_LINK_IFELSE([AC_LANG_CALL([], [pthread_join])], [ax_pthread_ok=yes]) + AC_MSG_RESULT([$ax_pthread_ok]) + if test "x$ax_pthread_ok" = "xno"; then + PTHREAD_LIBS="" + PTHREAD_CFLAGS="" + fi + CC="$ax_pthread_save_CC" + CFLAGS="$ax_pthread_save_CFLAGS" + LIBS="$ax_pthread_save_LIBS" +fi + +# We must check for the threads library under a number of different +# names; the ordering is very important because some systems +# (e.g. DEC) have both -lpthread and -lpthreads, where one of the +# libraries is broken (non-POSIX). + +# Create a list of thread flags to try. Items with a "," contain both +# C compiler flags (before ",") and linker flags (after ","). Other items +# starting with a "-" are C compiler flags, and remaining items are +# library names, except for "none" which indicates that we try without +# any flags at all, and "pthread-config" which is a program returning +# the flags for the Pth emulation library. + +ax_pthread_flags="pthreads none -Kthread -pthread -pthreads -mthreads pthread --thread-safe -mt pthread-config" + +# The ordering *is* (sometimes) important. Some notes on the +# individual items follow: + +# pthreads: AIX (must check this before -lpthread) +# none: in case threads are in libc; should be tried before -Kthread and +# other compiler flags to prevent continual compiler warnings +# -Kthread: Sequent (threads in libc, but -Kthread needed for pthread.h) +# -pthread: Linux/gcc (kernel threads), BSD/gcc (userland threads), Tru64 +# (Note: HP C rejects this with "bad form for `-t' option") +# -pthreads: Solaris/gcc (Note: HP C also rejects) +# -mt: Sun Workshop C (may only link SunOS threads [-lthread], but it +# doesn't hurt to check since this sometimes defines pthreads and +# -D_REENTRANT too), HP C (must be checked before -lpthread, which +# is present but should not be used directly; and before -mthreads, +# because the compiler interprets this as "-mt" + "-hreads") +# -mthreads: Mingw32/gcc, Lynx/gcc +# pthread: Linux, etcetera +# --thread-safe: KAI C++ +# pthread-config: use pthread-config program (for GNU Pth library) + +case $host_os in + + freebsd*) + + # -kthread: FreeBSD kernel threads (preferred to -pthread since SMP-able) + # lthread: LinuxThreads port on FreeBSD (also preferred to -pthread) + + ax_pthread_flags="-kthread lthread $ax_pthread_flags" + ;; + + hpux*) + + # From the cc(1) man page: "[-mt] Sets various -D flags to enable + # multi-threading and also sets -lpthread." + + ax_pthread_flags="-mt -pthread pthread $ax_pthread_flags" + ;; + + openedition*) + + # IBM z/OS requires a feature-test macro to be defined in order to + # enable POSIX threads at all, so give the user a hint if this is + # not set. (We don't define these ourselves, as they can affect + # other portions of the system API in unpredictable ways.) + + AC_EGREP_CPP([AX_PTHREAD_ZOS_MISSING], + [ +# if !defined(_OPEN_THREADS) && !defined(_UNIX03_THREADS) + AX_PTHREAD_ZOS_MISSING +# endif + ], + [AC_MSG_WARN([IBM z/OS requires -D_OPEN_THREADS or -D_UNIX03_THREADS to enable pthreads support.])]) + ;; + + solaris*) + + # On Solaris (at least, for some versions), libc contains stubbed + # (non-functional) versions of the pthreads routines, so link-based + # tests will erroneously succeed. (N.B.: The stubs are missing + # pthread_cleanup_push, or rather a function called by this macro, + # so we could check for that, but who knows whether they'll stub + # that too in a future libc.) So we'll check first for the + # standard Solaris way of linking pthreads (-mt -lpthread). + + ax_pthread_flags="-mt,-lpthread pthread $ax_pthread_flags" + ;; +esac + +# Are we compiling with Clang? + +AC_CACHE_CHECK([whether $CC is Clang], + [ax_cv_PTHREAD_CLANG], + [ax_cv_PTHREAD_CLANG=no + # Note that Autoconf sets GCC=yes for Clang as well as GCC + if test "x$GCC" = "xyes"; then + AC_EGREP_CPP([AX_PTHREAD_CC_IS_CLANG], + [/* Note: Clang 2.7 lacks __clang_[a-z]+__ */ +# if defined(__clang__) && defined(__llvm__) + AX_PTHREAD_CC_IS_CLANG +# endif + ], + [ax_cv_PTHREAD_CLANG=yes]) + fi + ]) +ax_pthread_clang="$ax_cv_PTHREAD_CLANG" + + +# GCC generally uses -pthread, or -pthreads on some platforms (e.g. SPARC) + +# Note that for GCC and Clang -pthread generally implies -lpthread, +# except when -nostdlib is passed. +# This is problematic using libtool to build C++ shared libraries with pthread: +# [1] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=25460 +# [2] https://bugzilla.redhat.com/show_bug.cgi?id=661333 +# [3] https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=468555 +# To solve this, first try -pthread together with -lpthread for GCC + +AS_IF([test "x$GCC" = "xyes"], + [ax_pthread_flags="-pthread,-lpthread -pthread -pthreads $ax_pthread_flags"]) + +# Clang takes -pthread (never supported any other flag), but we'll try with -lpthread first + +AS_IF([test "x$ax_pthread_clang" = "xyes"], + [ax_pthread_flags="-pthread,-lpthread -pthread"]) + + +# The presence of a feature test macro requesting re-entrant function +# definitions is, on some systems, a strong hint that pthreads support is +# correctly enabled + +case $host_os in + darwin* | hpux* | linux* | osf* | solaris*) + ax_pthread_check_macro="_REENTRANT" + ;; + + aix*) + ax_pthread_check_macro="_THREAD_SAFE" + ;; + + *) + ax_pthread_check_macro="--" + ;; +esac +AS_IF([test "x$ax_pthread_check_macro" = "x--"], + [ax_pthread_check_cond=0], + [ax_pthread_check_cond="!defined($ax_pthread_check_macro)"]) + + +if test "x$ax_pthread_ok" = "xno"; then +for ax_pthread_try_flag in $ax_pthread_flags; do + + case $ax_pthread_try_flag in + none) + AC_MSG_CHECKING([whether pthreads work without any flags]) + ;; + + *,*) + PTHREAD_CFLAGS=`echo $ax_pthread_try_flag | sed "s/^\(.*\),\(.*\)$/\1/"` + PTHREAD_LIBS=`echo $ax_pthread_try_flag | sed "s/^\(.*\),\(.*\)$/\2/"` + AC_MSG_CHECKING([whether pthreads work with "$PTHREAD_CFLAGS" and "$PTHREAD_LIBS"]) + ;; + + -*) + AC_MSG_CHECKING([whether pthreads work with $ax_pthread_try_flag]) + PTHREAD_CFLAGS="$ax_pthread_try_flag" + ;; + + pthread-config) + AC_CHECK_PROG([ax_pthread_config], [pthread-config], [yes], [no]) + AS_IF([test "x$ax_pthread_config" = "xno"], [continue]) + PTHREAD_CFLAGS="`pthread-config --cflags`" + PTHREAD_LIBS="`pthread-config --ldflags` `pthread-config --libs`" + ;; + + *) + AC_MSG_CHECKING([for the pthreads library -l$ax_pthread_try_flag]) + PTHREAD_LIBS="-l$ax_pthread_try_flag" + ;; + esac + + ax_pthread_save_CFLAGS="$CFLAGS" + ax_pthread_save_LIBS="$LIBS" + CFLAGS="$CFLAGS $PTHREAD_CFLAGS" + LIBS="$PTHREAD_LIBS $LIBS" + + # Check for various functions. We must include pthread.h, + # since some functions may be macros. (On the Sequent, we + # need a special flag -Kthread to make this header compile.) + # We check for pthread_join because it is in -lpthread on IRIX + # while pthread_create is in libc. We check for pthread_attr_init + # due to DEC craziness with -lpthreads. We check for + # pthread_cleanup_push because it is one of the few pthread + # functions on Solaris that doesn't have a non-functional libc stub. + # We try pthread_create on general principles. + + AC_LINK_IFELSE([AC_LANG_PROGRAM([#include +# if $ax_pthread_check_cond +# error "$ax_pthread_check_macro must be defined" +# endif + static void *some_global = NULL; + static void routine(void *a) + { + /* To avoid any unused-parameter or + unused-but-set-parameter warning. */ + some_global = a; + } + static void *start_routine(void *a) { return a; }], + [pthread_t th; pthread_attr_t attr; + pthread_create(&th, 0, start_routine, 0); + pthread_join(th, 0); + pthread_attr_init(&attr); + pthread_cleanup_push(routine, 0); + pthread_cleanup_pop(0) /* ; */])], + [ax_pthread_ok=yes], + []) + + CFLAGS="$ax_pthread_save_CFLAGS" + LIBS="$ax_pthread_save_LIBS" + + AC_MSG_RESULT([$ax_pthread_ok]) + AS_IF([test "x$ax_pthread_ok" = "xyes"], [break]) + + PTHREAD_LIBS="" + PTHREAD_CFLAGS="" +done +fi + + +# Clang needs special handling, because older versions handle the -pthread +# option in a rather... idiosyncratic way + +if test "x$ax_pthread_clang" = "xyes"; then + + # Clang takes -pthread; it has never supported any other flag + + # (Note 1: This will need to be revisited if a system that Clang + # supports has POSIX threads in a separate library. This tends not + # to be the way of modern systems, but it's conceivable.) + + # (Note 2: On some systems, notably Darwin, -pthread is not needed + # to get POSIX threads support; the API is always present and + # active. We could reasonably leave PTHREAD_CFLAGS empty. But + # -pthread does define _REENTRANT, and while the Darwin headers + # ignore this macro, third-party headers might not.) + + # However, older versions of Clang make a point of warning the user + # that, in an invocation where only linking and no compilation is + # taking place, the -pthread option has no effect ("argument unused + # during compilation"). They expect -pthread to be passed in only + # when source code is being compiled. + # + # Problem is, this is at odds with the way Automake and most other + # C build frameworks function, which is that the same flags used in + # compilation (CFLAGS) are also used in linking. Many systems + # supported by AX_PTHREAD require exactly this for POSIX threads + # support, and in fact it is often not straightforward to specify a + # flag that is used only in the compilation phase and not in + # linking. Such a scenario is extremely rare in practice. + # + # Even though use of the -pthread flag in linking would only print + # a warning, this can be a nuisance for well-run software projects + # that build with -Werror. So if the active version of Clang has + # this misfeature, we search for an option to squash it. + + AC_CACHE_CHECK([whether Clang needs flag to prevent "argument unused" warning when linking with -pthread], + [ax_cv_PTHREAD_CLANG_NO_WARN_FLAG], + [ax_cv_PTHREAD_CLANG_NO_WARN_FLAG=unknown + # Create an alternate version of $ac_link that compiles and + # links in two steps (.c -> .o, .o -> exe) instead of one + # (.c -> exe), because the warning occurs only in the second + # step + ax_pthread_save_ac_link="$ac_link" + ax_pthread_sed='s/conftest\.\$ac_ext/conftest.$ac_objext/g' + ax_pthread_link_step=`AS_ECHO(["$ac_link"]) | sed "$ax_pthread_sed"` + ax_pthread_2step_ac_link="($ac_compile) && (echo ==== >&5) && ($ax_pthread_link_step)" + ax_pthread_save_CFLAGS="$CFLAGS" + for ax_pthread_try in '' -Qunused-arguments -Wno-unused-command-line-argument unknown; do + AS_IF([test "x$ax_pthread_try" = "xunknown"], [break]) + CFLAGS="-Werror -Wunknown-warning-option $ax_pthread_try -pthread $ax_pthread_save_CFLAGS" + ac_link="$ax_pthread_save_ac_link" + AC_LINK_IFELSE([AC_LANG_SOURCE([[int main(void){return 0;}]])], + [ac_link="$ax_pthread_2step_ac_link" + AC_LINK_IFELSE([AC_LANG_SOURCE([[int main(void){return 0;}]])], + [break]) + ]) + done + ac_link="$ax_pthread_save_ac_link" + CFLAGS="$ax_pthread_save_CFLAGS" + AS_IF([test "x$ax_pthread_try" = "x"], [ax_pthread_try=no]) + ax_cv_PTHREAD_CLANG_NO_WARN_FLAG="$ax_pthread_try" + ]) + + case "$ax_cv_PTHREAD_CLANG_NO_WARN_FLAG" in + no | unknown) ;; + *) PTHREAD_CFLAGS="$ax_cv_PTHREAD_CLANG_NO_WARN_FLAG $PTHREAD_CFLAGS" ;; + esac + +fi # $ax_pthread_clang = yes + + + +# Various other checks: +if test "x$ax_pthread_ok" = "xyes"; then + ax_pthread_save_CFLAGS="$CFLAGS" + ax_pthread_save_LIBS="$LIBS" + CFLAGS="$CFLAGS $PTHREAD_CFLAGS" + LIBS="$PTHREAD_LIBS $LIBS" + + # Detect AIX lossage: JOINABLE attribute is called UNDETACHED. + AC_CACHE_CHECK([for joinable pthread attribute], + [ax_cv_PTHREAD_JOINABLE_ATTR], + [ax_cv_PTHREAD_JOINABLE_ATTR=unknown + for ax_pthread_attr in PTHREAD_CREATE_JOINABLE PTHREAD_CREATE_UNDETACHED; do + AC_LINK_IFELSE([AC_LANG_PROGRAM([#include ], + [int attr = $ax_pthread_attr; return attr /* ; */])], + [ax_cv_PTHREAD_JOINABLE_ATTR=$ax_pthread_attr; break], + []) + done + ]) + AS_IF([test "x$ax_cv_PTHREAD_JOINABLE_ATTR" != "xunknown" && \ + test "x$ax_cv_PTHREAD_JOINABLE_ATTR" != "xPTHREAD_CREATE_JOINABLE" && \ + test "x$ax_pthread_joinable_attr_defined" != "xyes"], + [AC_DEFINE_UNQUOTED([PTHREAD_CREATE_JOINABLE], + [$ax_cv_PTHREAD_JOINABLE_ATTR], + [Define to necessary symbol if this constant + uses a non-standard name on your system.]) + ax_pthread_joinable_attr_defined=yes + ]) + + AC_CACHE_CHECK([whether more special flags are required for pthreads], + [ax_cv_PTHREAD_SPECIAL_FLAGS], + [ax_cv_PTHREAD_SPECIAL_FLAGS=no + case $host_os in + solaris*) + ax_cv_PTHREAD_SPECIAL_FLAGS="-D_POSIX_PTHREAD_SEMANTICS" + ;; + esac + ]) + AS_IF([test "x$ax_cv_PTHREAD_SPECIAL_FLAGS" != "xno" && \ + test "x$ax_pthread_special_flags_added" != "xyes"], + [PTHREAD_CFLAGS="$ax_cv_PTHREAD_SPECIAL_FLAGS $PTHREAD_CFLAGS" + ax_pthread_special_flags_added=yes]) + + AC_CACHE_CHECK([for PTHREAD_PRIO_INHERIT], + [ax_cv_PTHREAD_PRIO_INHERIT], + [AC_LINK_IFELSE([AC_LANG_PROGRAM([[#include ]], + [[int i = PTHREAD_PRIO_INHERIT; + return i;]])], + [ax_cv_PTHREAD_PRIO_INHERIT=yes], + [ax_cv_PTHREAD_PRIO_INHERIT=no]) + ]) + AS_IF([test "x$ax_cv_PTHREAD_PRIO_INHERIT" = "xyes" && \ + test "x$ax_pthread_prio_inherit_defined" != "xyes"], + [AC_DEFINE([HAVE_PTHREAD_PRIO_INHERIT], [1], [Have PTHREAD_PRIO_INHERIT.]) + ax_pthread_prio_inherit_defined=yes + ]) + + CFLAGS="$ax_pthread_save_CFLAGS" + LIBS="$ax_pthread_save_LIBS" + + # More AIX lossage: compile with *_r variant + if test "x$GCC" != "xyes"; then + case $host_os in + aix*) + AS_CASE(["x/$CC"], + [x*/c89|x*/c89_128|x*/c99|x*/c99_128|x*/cc|x*/cc128|x*/xlc|x*/xlc_v6|x*/xlc128|x*/xlc128_v6], + [#handle absolute path differently from PATH based program lookup + AS_CASE(["x$CC"], + [x/*], + [ + AS_IF([AS_EXECUTABLE_P([${CC}_r])],[PTHREAD_CC="${CC}_r"]) + AS_IF([test "x${CXX}" != "x"], [AS_IF([AS_EXECUTABLE_P([${CXX}_r])],[PTHREAD_CXX="${CXX}_r"])]) + ], + [ + AC_CHECK_PROGS([PTHREAD_CC],[${CC}_r],[$CC]) + AS_IF([test "x${CXX}" != "x"], [AC_CHECK_PROGS([PTHREAD_CXX],[${CXX}_r],[$CXX])]) + ] + ) + ]) + ;; + esac + fi +fi + +test -n "$PTHREAD_CC" || PTHREAD_CC="$CC" +test -n "$PTHREAD_CXX" || PTHREAD_CXX="$CXX" + +AC_SUBST([PTHREAD_LIBS]) +AC_SUBST([PTHREAD_CFLAGS]) +AC_SUBST([PTHREAD_CC]) +AC_SUBST([PTHREAD_CXX]) + +# Finally, execute ACTION-IF-FOUND/ACTION-IF-NOT-FOUND: +if test "x$ax_pthread_ok" = "xyes"; then + ifelse([$1],,[AC_DEFINE([HAVE_PTHREAD],[1],[Define if you have POSIX threads libraries and header files.])],[$1]) + : +else + ax_pthread_ok=no + $2 +fi +AC_LANG_POP +])dnl AX_PTHREAD diff --git a/src/operations.c b/src/operations.c index 96fa4d88..f8c6fc10 100644 --- a/src/operations.c +++ b/src/operations.c @@ -1,6 +1,6 @@ /** - * Copyright (C) 2017, 2018, 2019, 2020, 2021 Genome Research Ltd. All - * rights reserved. + * Copyright (C) 2017, 2018, 2019, 2020, 2021, 2022 Genome Research + * Ltd. All rights reserved. * * 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 @@ -25,14 +25,49 @@ #include "baton.h" #include "operations.h" +// The connection used by iterate_json and connection_timeout +rcComm_t *connection; +// Mutex protecting the connection +pthread_mutex_t conn_mutex = PTHREAD_MUTEX_INITIALIZER; + +// While true, the client will refresh the connection +int connection_timeout_flag = 1; + +// Refresh the connection every timeout seconds +void *connection_timeout(void *timeout) { + int tsec = *((int *) timeout); + + struct timespec t; + t.tv_sec = tsec; + t.tv_nsec = 0; + + while (connection_timeout_flag) { + nanosleep(&t, NULL); + + logmsg(DEBUG, "Connection timeout of %d seconds reached", tsec); + pthread_mutex_lock(&conn_mutex); + if (connection) { + rcDisconnect(connection); + connection = NULL; + logmsg(DEBUG, "Closed the connection after timeout reached"); + } + pthread_mutex_unlock(&conn_mutex); + } + return 0; +} + static int iterate_json(FILE *input, rodsEnv *env, baton_json_op fn, operation_args_t *args, int *item_count, int *error_count) { - time_t connect_time = 0; - int reconnect = 0; // Set to 1 when reconnecting - rcComm_t *conn = NULL; - int drop_conn_count = 0; - int status = 0; + int status = 0; + int timeout = args->max_connect_time; + + pthread_t tid; + status = pthread_create(&tid, NULL, &connection_timeout, &timeout); + if (status != 0) { + logmsg(ERROR, "Failed to start connection management thread: %d", status); + goto finally; + } while (!exit_flag && !feof(input)) { size_t jflags = JSON_DISABLE_EOF_CHECK | JSON_REJECT_DUPLICATES; @@ -44,7 +79,6 @@ static int iterate_json(FILE *input, rodsEnv *env, baton_json_op fn, logmsg(ERROR, "JSON error at line %d, column %d: %s", load_error.line, load_error.column, load_error.text); } - continue; } @@ -56,54 +90,21 @@ static int iterate_json(FILE *input, rodsEnv *env, baton_json_op fn, continue; } - if (!conn) { - conn = rods_login(env); - if (!conn) { + pthread_mutex_lock(&conn_mutex); // Lock before connecting and executing a job + if (!connection) { + logmsg(DEBUG, "Opening a new connection"); + connection = rods_login(env); + if (!connection) { status = 1; + pthread_mutex_unlock(&conn_mutex); goto finally; } - - if (reconnect == 0) { - logmsg(INFO, "Connected to iRODS"); - } else { - logmsg(INFO, "Re-connected to iRODS"); - } - connect_time = time(0); - } - -#ifdef ENABLE_PUT_WORKAROUND - // If a put operation or dispatch to a put operation are - // requested, reconnect first. - int drop_conn = 0; - if (fn == baton_json_put_op) { - drop_conn = 1; - } - else if (fn == baton_json_dispatch_op) { - baton_error_t error; - const char *op = get_operation(item, &error); - // Ignore any error here because there are already checks - // for a valid operation string the dispatch function. We - // only want to know about the successful case at this - // point, so that we can decide if we have a put operation - // and thus need to reconnect first. - if (error.code == 0 && (str_equals(op, JSON_PUT_OP, MAX_STR_LEN))) { - drop_conn = 1; - } - } - - if (drop_conn) { - logmsg(INFO, "Reconnecting for put operation workaround"); - drop_conn_count++; - rcComm_t *newConn = rods_login(env); - if (!newConn) goto finally; - - rcDisconnect(conn); - conn = newConn; } -#endif baton_error_t error; - json_t *result = fn(env, conn, item, args, &error); + json_t *result = fn(env, connection, item, args, &error); + pthread_mutex_unlock(&conn_mutex); // Unlock before processing the result + if (error.code != 0) { // On error, add an error report to the input JSON as a // property and print the input JSON. A NULL result should @@ -141,17 +142,6 @@ static int iterate_json(FILE *input, rodsEnv *env, baton_json_op fn, (*item_count)++; json_decref(item); // JSON free - - time_t now = time(0); - double duration = difftime(now, connect_time); - if (args->max_connect_time > 0 && duration > args->max_connect_time) { - logmsg(INFO, "The connection to iRODS was open for %d seconds, " - "the maximum allowed is %d; closing the connection to " - "reopen a new one", duration, args->max_connect_time); - rcDisconnect(conn); - conn = NULL; - reconnect = 1; - } } // while if (exit_flag) { @@ -159,14 +149,17 @@ static int iterate_json(FILE *input, rodsEnv *env, baton_json_op fn, logmsg(WARN, "Exiting on signal with code %d", exit_flag); goto finally; } - - if (drop_conn_count > 0) { - logmsg(WARN, "Reconnected for put operations %d times", - drop_conn_count); - } finally: - if (conn) rcDisconnect(conn); + connection_timeout_flag = 0; + + pthread_mutex_lock(&conn_mutex); + if (connection) { + rcDisconnect(connection); + connection = NULL; + logmsg(DEBUG, "Closed the connection on exit") + } + pthread_mutex_unlock(&conn_mutex); return status; } @@ -189,7 +182,7 @@ int do_operation(FILE *input, baton_json_op fn, operation_args_t *args) { if (error_count > 0) { logmsg(WARN, "Processed %d items with %d errors", item_count, error_count); - status = 1; + status = 1; } else { logmsg(DEBUG, "Processed %d items with %d errors", diff --git a/src/operations.h b/src/operations.h index 9aa501f8..9686ee44 100644 --- a/src/operations.h +++ b/src/operations.h @@ -1,6 +1,6 @@ /** - * Copyright (C) 2013, 2014, 2015, 2016, 2017, 2018, 2019, 2021 Genome - * Research Ltd. All rights reserved. + * Copyright (C) 2013, 2014, 2015, 2016, 2017, 2018, 2019, 2021, 2022 + * Genome Research Ltd. All rights reserved. * * 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 @@ -22,6 +22,7 @@ #ifndef _BATON_OPERATIONS_H #define _BATON_OPERATIONS_H +#include #include #include From c9582912878a55f4bb3ab1e347fb4d2e2f965f9f Mon Sep 17 00:00:00 2001 From: Keith James Date: Wed, 27 Jul 2022 11:04:59 +0100 Subject: [PATCH 2/3] Fix timeout threading issues Incorporate suggestions kindly provided by Rob Davies: Protect the timeout thread flag with the available mutex. Use a condition variable to handle timeouts and completion better than a simple sleep. Clean up with pthread_join on exit, if a thread was created. Adjust the logging to include NOTICE level for key events. The logging is a bit excessive, but we can remove it later, when we've confirmed that all is behaving well. --- src/operations.c | 81 ++++++++++++++++++++++++++++++--------------- tests/check_baton.c | 7 ++-- 2 files changed, 58 insertions(+), 30 deletions(-) diff --git a/src/operations.c b/src/operations.c index f8c6fc10..baeeccc0 100644 --- a/src/operations.c +++ b/src/operations.c @@ -16,7 +16,7 @@ * along with this program. If not, see . * * @file operation.c - * @author Keith James + * @author Keith James , Rob Davies */ #include "config.h" @@ -25,34 +25,43 @@ #include "baton.h" #include "operations.h" +// Mutex protecting the connection and the run_timeout_thread flag +pthread_mutex_t conn_mutex = PTHREAD_MUTEX_INITIALIZER; // The connection used by iterate_json and connection_timeout rcComm_t *connection; -// Mutex protecting the connection -pthread_mutex_t conn_mutex = PTHREAD_MUTEX_INITIALIZER; - -// While true, the client will refresh the connection -int connection_timeout_flag = 1; +// While true, the client will continue to run the timeout thread +int run_timeout_thread = 1; +// Condition variable to exit the timeout thread when work is complete +pthread_cond_t watchdog_cond = PTHREAD_COND_INITIALIZER; // Refresh the connection every timeout seconds void *connection_timeout(void *timeout) { int tsec = *((int *) timeout); - struct timespec t; - t.tv_sec = tsec; - t.tv_nsec = 0; + struct timespec abs_timeout; - while (connection_timeout_flag) { - nanosleep(&t, NULL); - - logmsg(DEBUG, "Connection timeout of %d seconds reached", tsec); - pthread_mutex_lock(&conn_mutex); - if (connection) { - rcDisconnect(connection); - connection = NULL; - logmsg(DEBUG, "Closed the connection after timeout reached"); + pthread_mutex_lock(&conn_mutex); + while (run_timeout_thread) { + clock_gettime(CLOCK_REALTIME, &abs_timeout); + abs_timeout.tv_sec += tsec; + + int status; + do { + status = pthread_cond_timedwait(&watchdog_cond, &conn_mutex, + &abs_timeout); + } while (status == EINTR); + + if (status == ETIMEDOUT) { + if (connection) { + rcDisconnect(connection); + connection = NULL; + logmsg(NOTICE, "Closed the iRODS connection after a timeout " + "of %d seconds", tsec); + } } - pthread_mutex_unlock(&conn_mutex); } + pthread_mutex_unlock(&conn_mutex); + return 0; } @@ -61,11 +70,19 @@ static int iterate_json(FILE *input, rodsEnv *env, baton_json_op fn, int *item_count, int *error_count) { int status = 0; int timeout = args->max_connect_time; - pthread_t tid; - status = pthread_create(&tid, NULL, &connection_timeout, &timeout); - if (status != 0) { - logmsg(ERROR, "Failed to start connection management thread: %d", status); + int thread_status = -1; + + if (timeout < 10) { + logmsg(ERROR, "The connection timeout (--connect-time argument) " + "must be >=10 seconds"); + status = 1; + goto finally; + } + + thread_status = pthread_create(&tid, NULL, &connection_timeout, &timeout); + if (thread_status != 0) { + logmsg(ERROR, "Failed to start connection management thread: %d", thread_status); goto finally; } @@ -91,8 +108,9 @@ static int iterate_json(FILE *input, rodsEnv *env, baton_json_op fn, } pthread_mutex_lock(&conn_mutex); // Lock before connecting and executing a job + logmsg(DEBUG, "Work to do, lock obtained"); if (!connection) { - logmsg(DEBUG, "Opening a new connection"); + logmsg(NOTICE, "Opening a new iRODS connection"); connection = rods_login(env); if (!connection) { status = 1; @@ -104,6 +122,7 @@ static int iterate_json(FILE *input, rodsEnv *env, baton_json_op fn, baton_error_t error; json_t *result = fn(env, connection, item, args, &error); pthread_mutex_unlock(&conn_mutex); // Unlock before processing the result + logmsg(DEBUG, "Work done, lock released"); if (error.code != 0) { // On error, add an error report to the input JSON as a @@ -151,16 +170,24 @@ static int iterate_json(FILE *input, rodsEnv *env, baton_json_op fn, } finally: - connection_timeout_flag = 0; - pthread_mutex_lock(&conn_mutex); + run_timeout_thread = 0; + pthread_cond_signal(&watchdog_cond); // Unblock the thread waiting on cond + if (connection) { rcDisconnect(connection); connection = NULL; - logmsg(DEBUG, "Closed the connection on exit") + logmsg(NOTICE, "Closed the connection on exit") } pthread_mutex_unlock(&conn_mutex); + if (thread_status == 0) { + status = pthread_join(tid, NULL); + if (status != 0) { + logmsg(ERROR, "Timout thread failed to join: %s", strerror(status)); + } + } + return status; } diff --git a/tests/check_baton.c b/tests/check_baton.c index 69f22cb5..18665ba1 100644 --- a/tests/check_baton.c +++ b/tests/check_baton.c @@ -2444,9 +2444,10 @@ START_TEST(test_do_operation) { json_dumpf(obj1, json_tmp, 0); json_dumpf(obj2, json_tmp, 0); - operation_args_t args = { .flags = flags, - .buffer_size = buffer_size, - .zone_name = zone_name }; + operation_args_t args = { .flags = flags, + .buffer_size = buffer_size, + .zone_name = zone_name, + .max_connect_time = 10 }; for (int i = 0; i < num_ops; i++) { rewind(json_tmp); From 639b4d4a31ca4d7d18932e7467cae4cdc88fd24a Mon Sep 17 00:00:00 2001 From: mksanger Date: Thu, 4 Aug 2022 11:24:53 +0100 Subject: [PATCH 3/3] Update changelog for version 4.0.0 --- ChangeLog | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/ChangeLog b/ChangeLog index 1b2039df..78a1a8ca 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,10 @@ [Upcoming] + [4.0.0] + + Improve connection management by closing the connection while + waiting for the next JSON document and reopening when required. + [3.3.0] Fix ability to list the checksum of an object with a bad replica.