[PATCH 2/5] credential-cache: fix expiration calculation corner cases
- From
Jeff King <peff@peff.net>
- Date
- Sep 14, 2011, 19:17 UTC
- Message-ID
- <20110914191757.GB28267@sigill.intra.peff.net>
- In-Reply-To
- <20110914191704.GA23201@sigill.intra.peff.net>
The main credential-cache daemon loop calls poll to wait for a client or to trigger the expiration of credentials. When the last credential we hold expires, we exit.
However, there is a corner case: when we first start up, we have no credentials, and are waiting for a client to provide us with one. In this case, we ended up handing complete junk for the timeout argument to poll(). On some systems, this caused us to just wait a long time for the client (which usually showed up within a second or so). On OS X, however, the system quite reasonably complained about our junk value with EINVAL.
Fixing this is pretty straightforward; we just notice that we have no entries to compare against. However, that bug was covering up another one: our expiration calculation didn't give clients a chance to actually connect and provide us with a credential before we decided that we should exit because we weren't holding any credentials!
The new algorithm is:
1. Sleep until it's time to expire the most recent
credential. 2. If we don't have any credentials yet, wait 30 seconds
for a client to contact us and give us one. 3. After expiring the last credential, wait 30 seconds for
a client to provide us with one.Technically only parts (1) and (2) are needed to implement the original intended behavior.
But (3) is a minor optimization that is made easy by the new code. When a client gives us a credential, then removes it (e.g., because it had a bogus password), and then gives us another one, we used to exit, forcing the client to start a new daemon instance. Instead, we can just reuse the existing daemon instance.
Signed-off-by: Jeff King <peff@peff.net> --- credential-cache--daemon.c | 23 +++++++++++++++++++++-- 1 files changed, 21 insertions(+), 2 deletions(-)
diff --git a/credential-cache--daemon.c b/credential-cache--daemon.c index f520347..d6769b1 100644 --- a/credential-cache--daemon.c +++ b/credential-cache--daemon.c @@ -57,20 +57,33 @@ static void remove_credential(const struct credential *c) static int check_expirations(void) { + static unsigned long wait_for_entry_until; int i = 0; unsigned long now = time(NULL); unsigned long next = (unsigned long)-1; + /* + * Initially give the client 30 seconds to actually contact us + * and store a credential before we decide there's no point in + * keeping the daemon around. + */ + if (!wait_for_entry_until) + wait_for_entry_until = now + 30; + while (i < entries_nr) { if (entries[i].expiration <= now) { entries_nr--; - if (!entries_nr) - return 0; free(entries[i].item.description); free(entries[i].item.unique); free(entries[i].item.username); free(entries[i].item.password); memcpy(&entries[i], &entries[entries_nr], sizeof(*entries)); + /* + * Stick around 30 seconds in case a new credential + * shows up (e.g., because we just removed a failed + * one, and we will soon get the correct one). + */ + wait_for_entry_until = now + 30; } else { if (entries[i].expiration < next) @@ -79,6 +92,12 @@ static int check_expirations(void) } } + if (!entries_nr) { + if (wait_for_entry_until <= now) + return 0; + next = wait_for_entry_until; + } + return next - now; }
-- 1.7.6.252.ge9c18