[PATCH] !closing assertion on helper call comm_read during reconfigure

[Alex Rousskov]

Hello,

    While helper reading code does check for COMM_ERR_CLOSING, it is not
sufficient because helperReturnBuffer() called by that reading code may
notice the helper shutdown flag (set earlier by reconfigure) and start
closing the connection underneath the reading code feet.


HTH,

Alex.

Avoid !closing assertions when helpers call comm_read [during reconfigure].

While helper reading code does check for COMM_ERR_CLOSING, it is not sufficient
because helperReturnBuffer() called by the reading code may notice the helper 
shutdown flag (set earlier by reconfigure) and start closing the connection.

=== modified file 'src/helper.cc'
--- src/helper.cc	2013-03-14 23:04:37 +0000
+++ src/helper.cc	2013-05-10 23:59:57 +0000
@@ -19,40 +19,41 @@
  *  the Free Software Foundation; either version 2 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, write to the Free Software
  *  Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111, USA.
  *
  */
 
 #include "squid.h"
 #include "base/AsyncCbdataCalls.h"
 #include "comm.h"
 #include "comm/Connection.h"
 #include "comm/Write.h"
 #include "fd.h"
+#include "fde.h"
 #include "format/Quoting.h"
 #include "helper.h"
 #include "Mem.h"
 #include "MemBuf.h"
 #include "SquidIpc.h"
 #include "SquidMath.h"
 #include "SquidTime.h"
 #include "Store.h"
 #include "wordlist.h"
 
 #define HELPER_MAX_ARGS 64
 
 /** Initial Squid input buffer size. Helper responses may exceed this, and
  * Squid will grow the input buffer as needed, up to ReadBufMaxSize.
  */
 const size_t ReadBufMinSize(4*1024);
 
 /** Maximum safe size of a helper-to-Squid response message plus one.
  * Squid will warn and close the stream if a helper sends a too-big response.
  * ssl_crtd helper is known to produce responses of at least 10KB in size.
@@ -940,41 +941,41 @@
             // and remember that we have to skip forward 2 places now.
             skip = 2;
             --t;
         }
 
         *t = '\0';
 
         if (hlp->childs.concurrency) {
             i = strtol(msg, &msg, 10);
 
             while (*msg && xisspace(*msg))
                 ++msg;
         }
 
         helperReturnBuffer(i, srv, hlp, msg, t);
         srv->roffset -= (t - srv->rbuf) + skip;
         memmove(srv->rbuf, t + skip, srv->roffset);
         srv->rbuf[srv->roffset] = '\0';
     }
 
-    if (Comm::IsConnOpen(srv->readPipe)) {
+    if (Comm::IsConnOpen(srv->readPipe) && !fd_table[srv->readPipe->fd].closing()) {
         int spaceSize = srv->rbuf_sz - srv->roffset - 1;
         assert(spaceSize >= 0);
 
         // grow the input buffer if needed and possible
         if (!spaceSize && srv->rbuf_sz + 4096 <= ReadBufMaxSize) {
             srv->rbuf = (char *)memReallocBuf(srv->rbuf, srv->rbuf_sz + 4096, &srv->rbuf_sz);
             debugs(84, 3, HERE << "Grew read buffer to " << srv->rbuf_sz);
             spaceSize = srv->rbuf_sz - srv->roffset - 1;
             assert(spaceSize >= 0);
         }
 
         // quit reading if there is no space left
         if (!spaceSize) {
             debugs(84, DBG_IMPORTANT, "ERROR: Disconnecting from a " <<
                    "helper that overflowed " << srv->rbuf_sz << "-byte " <<
                    "Squid input buffer: " << hlp->id_name << " #" <<
                    (srv->index + 1));
             srv->closePipesSafely();
             return;
         }
@@ -1061,41 +1062,41 @@
         srv->roffset = 0;
         helperStatefulRequestFree(r);
         srv->request = NULL;
 
         -- srv->stats.pending;
         ++ srv->stats.replies;
 
         ++ hlp->stats.replies;
         srv->answer_time = current_time;
         hlp->stats.avg_svc_time =
             Math::intAverage(hlp->stats.avg_svc_time,
                              tvSubMsec(srv->dispatch_time, current_time),
                              hlp->stats.replies, REDIRECT_AV_FACTOR);
 
         if (called)
             helperStatefulServerDone(srv);
         else
             helperStatefulReleaseServer(srv);
     }
 
-    if (Comm::IsConnOpen(srv->readPipe)) {
+    if (Comm::IsConnOpen(srv->readPipe) && !fd_table[srv->readPipe->fd].closing()) {
         int spaceSize = srv->rbuf_sz - srv->roffset - 1;
         assert(spaceSize >= 0);
 
         // grow the input buffer if needed and possible
         if (!spaceSize && srv->rbuf_sz + 4096 <= ReadBufMaxSize) {
             srv->rbuf = (char *)memReallocBuf(srv->rbuf, srv->rbuf_sz + 4096, &srv->rbuf_sz);
             debugs(84, 3, HERE << "Grew read buffer to " << srv->rbuf_sz);
             spaceSize = srv->rbuf_sz - srv->roffset - 1;
             assert(spaceSize >= 0);
         }
 
         // quit reading if there is no space left
         if (!spaceSize) {
             debugs(84, DBG_IMPORTANT, "ERROR: Disconnecting from a " <<
                    "helper that overflowed " << srv->rbuf_sz << "-byte " <<
                    "Squid input buffer: " << hlp->id_name << " #" <<
                    (srv->index + 1));
             srv->closePipesSafely();
             return;
         }