Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix requests processing loop to honor KeepAliveTimeout #46

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Commits on Oct 27, 2022

  1. Fix requests processing loop to honor KeepAliveTimeout

    Admin is allowed to configure a KeepAliveTimeout:
      The maximum time in seconds waiting for a new requests before a
      connection will be closed.
      If the value is set to -1, there is no timeout.
    
    The processing loop calls keepalive_request() to implement the wait;
    'keepalive_request' can return either due to a inactivity timeout, or
    upon connection close/error, or due to a new request pending.
    
    Alas, the return code isn't checked, and the code always continues to
    beginning of loop, invoking a new 'process_request()' call.
    
    The problem is that in a *genuine* timeout event (no further exchanges
    from the icap client on the connection), continuing to process_request
    will cause an *additional* blocking wait:
      process_request()
        do_request()
          parse_header()
            ci_read_icap_header(req, h, TIMEOUT)  # defaults to 300s
              wait_for_data(req->connection, timeout, ci_wait_for_read)
    
    which means the KeepAliveTimeout value isn't really honored, and the
    worker thread will stall for an additional timeout (either until new
    request arrives or until client closed the connection, or timeout).
    
    Fix, by testing keepalive_request() return code: if it is not positive
    (i.e. timeout or error), just break out of the requests processing loop,
    which will close the connection.
    shmull committed Oct 27, 2022
    Configuration menu
    Copy the full SHA
    dbe67ab View commit details
    Browse the repository at this point in the history