OpenAFS Master Repository branch, openafs-stable-1_8_x, updated. openafs-stable-1_8_2-24-g036c01d

Gerrit Code Review gerrit@openafs.org
Fri, 25 Jan 2019 09:29:24 -0500


The following commit has been merged in the openafs-stable-1_8_x branch:
commit 036c01d2f129b7118a77ecd6d89fbc779d91c224
Author: Andrew Deason <adeason@sinenomine.net>
Date:   Thu Nov 2 16:41:52 2017 -0500

    rx: Convert rxinit_status to rx_IsRunning()
    
    Currently, all rx code examines the atomic rxinit_status to determine
    if rx is running (that is, if rx_InitHost has been called, and
    rx_Finalize/shutdown_rx hasn't been called). This is used in rx.c to
    see if we're redundantly calling our setup/teardown functions, and
    outside of rx.c in a couple of places to see if rx-related resources
    have been initialized.
    
    The usage of rxinit_status is a little confusing, since setting bit 0
    indicates that rx is not running, and clearing bit 0 indicates rx is
    running. Since using rxinit_status requires atomic functions, this
    makes code checking or setting rxinit_status a little verbose, and it
    can be hard to see what it is checking for. (For example, does
    'if (!rx_atomic_test_and_clear_bit(&rxinit_status, 0))' succeed when
    rx running, or when rx is not running?)
    
    The current usage of rxinit_status in rx_InitHost also does not handle
    initialization errors correctly. rx_InitHost clears rxinit_status near
    the beginning of the function, but does not set rxinit_status if an
    error is encountered. This means that any code that checks
    rxinit_status (such as another rx_InitHost call) will think that rx
    was initialized successfully, but various resources aren't actually
    setup. This can cause segfaults and other errors as the code tries to
    actually use rx.
    
    This can easily be seen in bosserver, if bosserver is started up while
    the local host/port is in use by someone else. bosserver will try to
    rx_InitHost, which will fail, and then we'll try to rx_InitHost again,
    which will immediately succeed without doing any init. We then
    segfault quickly afterwards as we try to use unitialized rx resources.
    
    To fix all of this, refactor code using rxinit_status to use a new
    function, called rx_IsRunning(), to make it a little clearer what
    we're checking for. We also re-introduce the LOCK_RX_INIT locks to
    prevent functions like rx_InitHost and rx_Finalize from running in
    parallel.
    
    Note that non-init/shutdown code (such as rx_upcall or rx_GetIFInfo)
    does not need to wait for LOCK_RX_INIT to check if rx is running or
    not. These functions only care if rx is currently setup enough to be
    used, so we can immediately return a 'yes' or 'no' answer. That is, if
    rx_InitHost is in the middle of running, rx_IsRunning returns 0, since
    some resouces may not be fully initialized.
    
    Reviewed-on: https://gerrit.openafs.org/12761
    Reviewed-by: Michael Meffie <mmeffie@sinenomine.net>
    Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
    Tested-by: BuildBot <buildbot@rampaginggeek.com>
    (cherry picked from commit 5ced6025b9f11fadbdf2e092bf40cc87499ed277)
    
    Change-Id: I38ef9e3aea8a1f20e9db488a44da4535f76432d1
    Reviewed-on: https://gerrit.openafs.org/13416
    Tested-by: BuildBot <buildbot@rampaginggeek.com>
    Reviewed-by: Michael Meffie <mmeffie@sinenomine.net>
    Reviewed-by: Cheyenne Wills <cwills@sinenomine.net>
    Reviewed-by: Andrew Deason <adeason@sinenomine.net>
    Reviewed-by: Stephan Wiesand <stephan.wiesand@desy.de>

 src/rx/DARWIN/rx_knet.c |    4 +--
 src/rx/rx.c             |   87 ++++++++++++++++++++++++++++++++++++-----------
 src/rx/rx_internal.h    |    1 +
 src/rx/rx_user.c        |    7 ++--
 4 files changed, 72 insertions(+), 27 deletions(-)

-- 
OpenAFS Master Repository