From f8ff8237ecd30410bf6fe924f58f9ed05132e99b Mon Sep 17 00:00:00 2001 From: Amos Jeffries Date: Fri, 14 Aug 2009 16:50:47 +1200 Subject: [PATCH] Author: Henrik Nordstrom Remove support for deferred state in stateful helpers the deferred state were previously used for the challenge reuse mode in NTLM, but is since long unused as it was both incompatible with NTLMv2 and also not very stable for authentication. --- src/enums.h | 11 +- src/helper.cc | 288 ++++++-------------------------------------------- src/helper.h | 29 ++--- 3 files changed, 43 insertions(+), 285 deletions(-) diff --git a/src/enums.h b/src/enums.h index 6748079132..d43cd53f62 100755 --- a/src/enums.h +++ b/src/enums.h @@ -374,18 +374,9 @@ typedef enum { typedef enum { S_HELPER_UNKNOWN, S_HELPER_RESERVE, - S_HELPER_RELEASE, - S_HELPER_DEFER + S_HELPER_RELEASE } stateful_helper_callback_t; -/* stateful helper reservation info */ -typedef enum { - S_HELPER_FREE, /* available for requests */ - S_HELPER_RESERVED, /* in a reserved state - no active request, but state data in the helper shouldn't be disturbed */ - S_HELPER_DEFERRED /* available for requests, and at least one more will come from a previous caller with the server pointer */ -} stateful_helper_reserve_t; - - #if SQUID_SNMP enum { SNMP_C_VIEW, diff --git a/src/helper.cc b/src/helper.cc index aaeeb1d60c..959edaf6ed 100644 --- a/src/helper.cc +++ b/src/helper.cc @@ -56,12 +56,10 @@ static void helperDispatch(helper_server * srv, helper_request * r); static void helperStatefulDispatch(helper_stateful_server * srv, helper_stateful_request * r); static void helperKickQueue(helper * hlp); static void helperStatefulKickQueue(statefulhelper * hlp); +static void helperStatefulServerDone(helper_stateful_server * srv); static void helperRequestFree(helper_request * r); static void helperStatefulRequestFree(helper_stateful_request * r); static void StatefulEnqueue(statefulhelper * hlp, helper_stateful_request * r); -static helper_stateful_request *StatefulServerDequeue(helper_stateful_server * srv); -static void StatefulServerEnqueue(helper_stateful_server * srv, helper_stateful_request * r); -static void helperStatefulServerKickQueue(helper_stateful_server * srv); static bool helperStartStats(StoreEntry *sentry, void *hlp, const char *label); @@ -249,10 +247,7 @@ helperStatefulOpenServers(statefulhelper * hlp) helper_stateful_server *srv = cbdataAlloc(helper_stateful_server); srv->hIpc = hIpc; srv->pid = pid; - srv->flags.reserved = S_HELPER_FREE; - srv->deferred_requests = 0; - srv->stats.deferbyfunc = 0; - srv->stats.deferbycb = 0; + srv->flags.reserved = 0; srv->stats.submits = 0; srv->stats.releases = 0; srv->index = k; @@ -319,8 +314,7 @@ helperSubmit(helper * hlp, const char *buf, HLPCB * callback, void *data) debugs(84, 9, "helperSubmit: " << buf); } -/* lastserver = "server last used as part of a deferred or reserved - * request sequence" +/* lastserver = "server last used as part of a reserved request sequence" */ void helperStatefulSubmit(statefulhelper * hlp, const char *buf, HLPSCB * callback, void *data, helper_stateful_server * lastserver) @@ -346,28 +340,12 @@ helperStatefulSubmit(statefulhelper * hlp, const char *buf, HLPSCB * callback, v if ((buf != NULL) && lastserver) { debugs(84, 5, "StatefulSubmit with lastserver " << lastserver); - /* the queue doesn't count for this assert because queued requests - * have already gone through here and been tested. - * It's legal to have deferred_requests == 0 and queue entries - * and status of S_HELPEER_DEFERRED. - * BUT: It's not legal to submit a new request w/lastserver in - * that state. - */ - assert(!(lastserver->deferred_requests == 0 && - lastserver->flags.reserved == S_HELPER_DEFERRED)); - if (lastserver->flags.reserved != S_HELPER_RESERVED) { - lastserver->stats.submits++; - lastserver->deferred_requests--; - } + assert(lastserver->flags.reserved); + assert(!(lastserver->request)); - if (!(lastserver->request)) { - debugs(84, 5, "StatefulSubmit dispatching"); - helperStatefulDispatch(lastserver, r); - } else { - debugs(84, 5, "StatefulSubmit queuing"); - StatefulServerEnqueue(lastserver, r); - } + debugs(84, 5, "StatefulSubmit dispatching"); + helperStatefulDispatch(lastserver, r); } else { helper_stateful_server *srv; if ((srv = StatefulGetFirstAvailable(hlp))) { @@ -379,150 +357,26 @@ helperStatefulSubmit(statefulhelper * hlp, const char *buf, HLPSCB * callback, v debugs(84, 9, "helperStatefulSubmit: placeholder: '" << r->placeholder << "', buf '" << buf << "'."); } -/* - * helperStatefulDefer - * - * find and add a deferred request to a helper - */ -helper_stateful_server * -helperStatefulDefer(statefulhelper * hlp) -{ - if (hlp == NULL) { - debugs(84, 3, "helperStatefulDefer: hlp == NULL"); - return NULL; - } - - debugs(84, 5, "helperStatefulDefer: Running servers " << hlp->n_running); - - if (hlp->n_running == 0) { - debugs(84, 1, "helperStatefulDefer: No running servers!. "); - return NULL; - } - - helper_stateful_server *rv = StatefulGetFirstAvailable(hlp); - - if (rv == NULL) { - /* - * all currently busy; loop through servers and find server - * with the shortest queue - */ - - for (dlink_node *n = hlp->servers.head; n != NULL; n = n->next) { - helper_stateful_server *srv = (helper_stateful_server *)n->data; - - if (srv->flags.reserved == S_HELPER_RESERVED) - continue; - - if (!srv->flags.shutdown) - continue; - - if ((hlp->IsAvailable != NULL) && (srv->data != NULL) && - !(hlp->IsAvailable(srv->data))) - continue; - - if ((rv != NULL) && (rv->deferred_requests < srv->deferred_requests)) - continue; - - rv = srv; - } - } - - if (rv == NULL) { - debugs(84, 1, "helperStatefulDefer: None available."); - return NULL; - } - - /* consistency check: - * when the deferred count is 0, - * submits + releases == deferbyfunc + deferbycb - * Or in english, when there are no deferred requests, the amount - * we have submitted to the queue or cancelled must equal the amount - * we have said we wanted to be able to submit or cancel - */ - if (rv->deferred_requests == 0) - assert(rv->stats.submits + rv->stats.releases == - rv->stats.deferbyfunc + rv->stats.deferbycb); - - rv->flags.reserved = S_HELPER_DEFERRED; - - rv->deferred_requests++; - - rv->stats.deferbyfunc++; - - return rv; -} - -void -helperStatefulReset(helper_stateful_server * srv) -/* puts this helper back in the queue. the calling app is required to - * manage the state in the helper. - */ -{ - statefulhelper *hlp = srv->parent; - helper_stateful_request *r = srv->request; - - if (r != NULL) { - /* reset attempt DURING an outstaning request */ - debugs(84, 1, "helperStatefulReset: RESET During request " << hlp->id_name << " "); - srv->flags.busy = 0; - srv->roffset = 0; - helperStatefulRequestFree(r); - srv->request = NULL; - } - - srv->flags.busy = 0; - - if (srv->queue.head) { - srv->flags.reserved = S_HELPER_DEFERRED; - } else { - srv->flags.reserved = S_HELPER_FREE; - - if ((srv->parent->OnEmptyQueue != NULL) && (srv->data)) - srv->parent->OnEmptyQueue(srv->data); - } - - helperStatefulServerKickQueue(srv); -} - /* * DPW 2007-05-08 * * helperStatefulReleaseServer tells the helper that whoever was * using it no longer needs its services. - * - * If the state is S_HELPER_DEFERRED, decrease the deferred count. - * If the count goes to zero, then it can become S_HELPER_FREE. - * - * If the state is S_HELPER_RESERVED, then it should always - * become S_HELPER_FREE. */ void helperStatefulReleaseServer(helper_stateful_server * srv) { debugs(84, 3, HERE << "srv-" << srv->index << " flags.reserved = " << srv->flags.reserved); - if (srv->flags.reserved == S_HELPER_FREE) + if (!srv->flags.reserved) return; srv->stats.releases++; + srv->flags.reserved = 0; - if (srv->flags.reserved == S_HELPER_DEFERRED) { - assert(srv->deferred_requests); - srv->deferred_requests--; - if (srv->deferred_requests) { - debugs(0,0,HERE << "helperStatefulReleaseServer srv->deferred_requests=" << srv->deferred_requests); - return; - } - if (srv->queue.head) { - debugs(0,0,HERE << "helperStatefulReleaseServer srv->queue.head not NULL"); - return; - } - } - - srv->flags.reserved = S_HELPER_FREE; if (srv->parent->OnEmptyQueue != NULL && srv->data) srv->parent->OnEmptyQueue(srv->data); - helperStatefulServerKickQueue(srv); + helperStatefulServerDone(srv); } void * @@ -604,12 +458,11 @@ helperStatefulStats(StoreEntry * sentry, statefulhelper * hlp, const char *label storeAppendPrintf(sentry, "avg service time: %d msec\n", hlp->stats.avg_svc_time); storeAppendPrintf(sentry, "\n"); - storeAppendPrintf(sentry, "%7s\t%7s\t%7s\t%11s\t%20s\t%s\t%7s\t%7s\t%7s\n", + storeAppendPrintf(sentry, "%7s\t%7s\t%7s\t%11s\t%6s\t%7s\t%7s\t%7s\n", "#", "FD", "PID", "# Requests", - "# Deferred Requests", "Flags", "Time", "Offset", @@ -617,17 +470,15 @@ helperStatefulStats(StoreEntry * sentry, statefulhelper * hlp, const char *label for (dlink_node *link = hlp->servers.head; link; link = link->next) { helper_stateful_server *srv = (helper_stateful_server *)link->data; - double tt = 0.001 * tvSubMsec(srv->dispatch_time, - srv->flags.busy ? current_time : srv->answer_time); - storeAppendPrintf(sentry, "%7d\t%7d\t%7d\t%11d\t%20d\t%c%c%c%c%c\t%7.3f\t%7d\t%s\n", + double tt = 0.001 * tvSubMsec(srv->dispatch_time, srv->flags.busy ? current_time : srv->answer_time); + storeAppendPrintf(sentry, "%7d\t%7d\t%7d\t%11d\t%c%c%c%c%c\t%7.3f\t%7d\t%s\n", srv->index + 1, srv->rfd, srv->pid, srv->stats.uses, - (int) srv->deferred_requests, srv->flags.busy ? 'B' : ' ', srv->flags.closing ? 'C' : ' ', - srv->flags.reserved == S_HELPER_RESERVED ? 'R' : (srv->flags.reserved == S_HELPER_DEFERRED ? 'D' : ' '), + srv->flags.reserved ? 'R' : ' ', srv->flags.shutdown ? 'S' : ' ', srv->request ? (srv->request->placeholder ? 'P' : ' ') : ' ', tt < 0.0 ? 0.0 : tt, @@ -638,7 +489,7 @@ helperStatefulStats(StoreEntry * sentry, statefulhelper * hlp, const char *label storeAppendPrintf(sentry, "\nFlags key:\n\n"); storeAppendPrintf(sentry, " B = BUSY\n"); storeAppendPrintf(sentry, " C = CLOSING\n"); - storeAppendPrintf(sentry, " R = RESERVED or DEFERRED\n"); + storeAppendPrintf(sentry, " R = RESERVED\n"); storeAppendPrintf(sentry, " S = SHUTDOWN PENDING\n"); storeAppendPrintf(sentry, " P = PLACEHOLDER\n"); } @@ -746,14 +597,14 @@ helperStatefulShutdown(statefulhelper * hlp) continue; } - if (srv->flags.reserved != S_HELPER_FREE) { - debugs(84, 3, "helperStatefulShutdown: " << hlp->id_name << " #" << srv->index + 1 << " is RESERVED."); - continue; - } - - if (srv->deferred_requests) { - debugs(84, 3, "helperStatefulShutdown: " << hlp->id_name << " #" << srv->index + 1 << " has DEFERRED requests."); - continue; + if (srv->flags.reserved) { + if (shutting_down) { + debugs(84, 3, "helperStatefulShutdown: " << hlp->id_name << " #" << srv->index + 1 << " is RESERVED. Closing anyway."); + } + else { + debugs(84, 3, "helperStatefulShutdown: " << hlp->id_name << " #" << srv->index + 1 << " is RESERVED. Not Shutting Down Yet."); + continue; + } } srv->flags.closing = 1; @@ -1156,44 +1007,22 @@ helperStatefulHandleRead(int fd, char *buf, size_t len, comm_err_t flag, int xer case S_HELPER_RELEASE: /* helper finished with */ - if (!srv->deferred_requests && !srv->queue.head) { - srv->flags.reserved = S_HELPER_FREE; + srv->flags.reserved = 0; - if ((srv->parent->OnEmptyQueue != NULL) && (srv->data)) - srv->parent->OnEmptyQueue(srv->data); + if ((srv->parent->OnEmptyQueue != NULL) && (srv->data)) + srv->parent->OnEmptyQueue(srv->data); - debugs(84, 5, "StatefulHandleRead: releasing " << hlp->id_name << " #" << srv->index + 1); - } else { - srv->flags.reserved = S_HELPER_DEFERRED; - debugs(84, 5, "StatefulHandleRead: outstanding deferred requests on " << - hlp->id_name << " #" << srv->index + 1 << - ". reserving for deferred requests."); - } + debugs(84, 5, "StatefulHandleRead: releasing " << hlp->id_name << " #" << srv->index + 1); break; case S_HELPER_RESERVE: /* 'pin' this helper for the caller */ - if (!srv->queue.head) { - assert(srv->deferred_requests == 0); - srv->flags.reserved = S_HELPER_RESERVED; - debugs(84, 5, "StatefulHandleRead: reserving " << hlp->id_name << " #" << srv->index + 1); - } else { - fatal("StatefulHandleRead: Callback routine attempted to reserve a stateful helper with deferred requests. This can lead to deadlock.\n"); - } + srv->flags.reserved = 1; + debugs(84, 5, "StatefulHandleRead: reserving " << hlp->id_name << " #" << srv->index + 1); break; - case S_HELPER_DEFER: - /* the helper is still needed, but can - * be used for other requests in the meantime. - */ - srv->flags.reserved = S_HELPER_DEFERRED; - srv->deferred_requests++; - srv->stats.deferbycb++; - debugs(84, 5, "StatefulHandleRead: reserving " << hlp->id_name << " #" << srv->index + 1 << " for deferred requests."); - break; - default: fatal("helperStatefulHandleRead: unknown stateful helper callback result.\n"); } @@ -1213,7 +1042,7 @@ helperStatefulHandleRead(int fd, char *buf, size_t len, comm_err_t flag, int xer tvSubMsec(srv->dispatch_time, current_time), hlp->stats.replies, REDIRECT_AV_FACTOR); - helperStatefulServerKickQueue(srv); + helperStatefulServerDone(srv); } if (srv->rfd != -1) @@ -1278,31 +1107,6 @@ StatefulEnqueue(statefulhelper * hlp, helper_stateful_request * r) } -static void -StatefulServerEnqueue(helper_stateful_server * srv, helper_stateful_request * r) -{ - dlink_node *link = (dlink_node *)memAllocate(MEM_DLINK_NODE); - dlinkAddTail(r, link, &srv->queue); - /* TODO: warning if the queue on this server is more than X - * We don't check the queue size at the moment, because - * requests hitting here are deferrable - */ - /* hlp->stats.queue_size++; - * if (hlp->stats.queue_size < hlp->n_running) - * return; - * if (squid_curtime - hlp->last_queue_warn < 600) - * return; - * if (shutting_down || reconfiguring) - * return; - * hlp->last_queue_warn = squid_curtime; - * debugs(84, 0, "WARNING: All " << hlp->id_name << " processes are busy."); - * debugs(84, 0, "WARNING: " << hlp->stats.queue_size << " pending requests queued"); - * if (hlp->stats.queue_size > hlp->n_running * 2) - * fatalf("Too many queued %s requests", hlp->id_name); - * debugs(84, 1, "Consider increasing the number of " << hlp->id_name << " processes in your config file." ); */ -} - - static helper_request * Dequeue(helper * hlp) { @@ -1319,21 +1123,6 @@ Dequeue(helper * hlp) return r; } -static helper_stateful_request * -StatefulServerDequeue(helper_stateful_server * srv) -{ - dlink_node *link; - helper_stateful_request *r = NULL; - - if ((link = srv->queue.head)) { - r = (helper_stateful_request *)link->data; - dlinkDelete(link, &srv->queue); - memFree(link, MEM_DLINK_NODE); - } - - return r; -} - static helper_stateful_request * StatefulDequeue(statefulhelper * hlp) { @@ -1407,7 +1196,7 @@ StatefulGetFirstAvailable(statefulhelper * hlp) if (srv->flags.busy) continue; - if (srv->flags.reserved == S_HELPER_RESERVED) + if (srv->flags.reserved) continue; if (srv->flags.shutdown) @@ -1527,7 +1316,7 @@ helperStatefulDispatch(helper_stateful_server * srv, helper_stateful_request * r if (r->placeholder == 1) { /* a callback is needed before this request can _use_ a helper. */ - /* we don't care about releasing/deferring this helper. The request NEVER + /* we don't care about releasing this helper. The request NEVER * gets to the helper. So we throw away the return code */ r->callback(r->data, srv, NULL); /* throw away the placeholder */ @@ -1536,7 +1325,7 @@ helperStatefulDispatch(helper_stateful_server * srv, helper_stateful_request * r * request to the helper which is why we test for the request*/ if (srv->request == NULL) - helperStatefulServerKickQueue(srv); + helperStatefulServerDone(srv); return; } @@ -1579,18 +1368,11 @@ helperStatefulKickQueue(statefulhelper * hlp) } static void -helperStatefulServerKickQueue(helper_stateful_server * srv) +helperStatefulServerDone(helper_stateful_server * srv) { - helper_stateful_request *r; - - if ((r = StatefulServerDequeue(srv))) { - helperStatefulDispatch(srv, r); - return; - } - if (!srv->flags.shutdown) { helperStatefulKickQueue(srv->parent); - } else if (!srv->flags.closing && srv->flags.reserved == S_HELPER_FREE && !srv->flags.busy) { + } else if (!srv->flags.closing && !srv->flags.reserved && !srv->flags.busy) { int wfd = srv->wfd; srv->wfd = -1; if (srv->rfd == wfd) diff --git a/src/helper.h b/src/helper.h index f5ed077a11..8de49dd716 100644 --- a/src/helper.h +++ b/src/helper.h @@ -171,37 +171,24 @@ struct _helper_stateful_server struct timeval answer_time; dlink_node link; - dlink_list queue; statefulhelper *parent; helper_stateful_request *request; struct _helper_stateful_flags { - -unsigned int busy: - 1; - -unsigned int closing: - 1; - -unsigned int shutdown: - 1; - stateful_helper_reserve_t reserved; - } - - flags; + unsigned int busy:1; + unsigned int closing:1; + unsigned int shutdown:1; + unsigned int reserved:1; + } flags; struct { int uses; int submits; int releases; - int deferbyfunc; - int deferbycb; - } + } stats; - stats; - int deferred_requests; /* current number of deferred requests */ void *data; /* State data used by the calling routines */ void *hIpc; }; @@ -227,7 +214,7 @@ public: MEMPROXY_CLASS(helper_stateful_request); char *buf; HLPSCB *callback; - int placeholder; /* if 1, this is a dummy request waiting for a stateful helper to become available for deferred requests.*/ + int placeholder; /* if 1, this is a dummy request waiting for a stateful helper to become available */ void *data; }; @@ -246,10 +233,8 @@ SQUIDCEXTERN helper *helperCreate(const char *); SQUIDCEXTERN statefulhelper *helperStatefulCreate(const char *); SQUIDCEXTERN void helperFree(helper *); SQUIDCEXTERN void helperStatefulFree(statefulhelper *); -SQUIDCEXTERN void helperStatefulReset(helper_stateful_server * srv); SQUIDCEXTERN void helperStatefulReleaseServer(helper_stateful_server * srv); SQUIDCEXTERN void *helperStatefulServerGetData(helper_stateful_server * srv); -SQUIDCEXTERN helper_stateful_server *helperStatefulDefer(statefulhelper *); -- 2.47.2