diff --git a/src/BodyPipe.cc b/src/BodyPipe.cc index dfdf53ae1c9..3b5a8fbe4ba 100644 --- a/src/BodyPipe.cc +++ b/src/BodyPipe.cc @@ -266,18 +266,18 @@ BodyPipe::clearConsumer() void BodyPipe::expectNoConsumption() { - // We may be called multiple times because multiple jobs on the consumption - // chain may realize that there will be no more setConsumer() calls (e.g., - // consuming code and retrying code). It is both difficult and not really - // necessary for them to coordinate their expectNoConsumption() calls. - - // As a consequence, we may be called when we are auto-consuming already. - - if (!abortedConsumption && !exhausted()) { - // Before we abort, any regular consumption should be over and auto - // consumption must not be started. - Must(!theConsumer); - + // We and enableAutoConsumption() may be called multiple times because + // multiple jobs on the consumption chain may realize that there will be no + // more setConsumer() calls (e.g., consuming code and retrying code). It is + // both difficult and unnecessary for them to coordinate their calls. + + // As a consequence, we may be called when already auto-consuming, including + // cases where abortedConsumption is still false. We could try to harden + // this by also aborting consumption from enableAutoConsumption() when there + // is no consumer, but see errorAppendEntry() TODO for a better plan. + + debugs(91, 7, status()); + if (!abortedConsumption && !exhausted() && !theConsumer) { AsyncCall::Pointer call= asyncCall(91, 7, "BodyProducer::noteBodyConsumerAborted", BodyProducerDialer(theProducer, diff --git a/src/errorpage.cc b/src/errorpage.cc index de019667325..bbbfc7acde5 100644 --- a/src/errorpage.cc +++ b/src/errorpage.cc @@ -741,6 +741,29 @@ errorAppendEntry(StoreEntry * entry, ErrorState * err) assert (entry->isEmpty()); debugs(4, 4, "storing " << err << " in " << *entry); + if (const auto &request = err->request) { + if (const auto &bodyPipe = request->body_pipe) { + // We cannot expectNoConsumption() here (yet): This request may be a + // virgin request being consumed by adaptation that should continue + // even in error-handling cases. startAutoConsumptionIfNeeded() call + // triggered by enableAutoConsumption() below skips such requests. + // + // Today, we also cannot enableAutoConsumption() earlier because it + // could result in premature consumption in BodyPipe::postAppend() + // followed by an unwanted setConsumerIfNotLate() failure. + // + // TODO: Simplify BodyPipe auto-consumption by automatically + // enabling it when no new consumers are expected, removing the need + // for explicit enableAutoConsumption() calls like the one below. + // + // Code like clientReplyContext::sendClientOldEntry() might use + // another StoreEntry for this master transaction, but we want to + // consume this request body even in those hypothetical error cases + // to prevent stuck (client-Squid or REQMOD) transactions. + bodyPipe->enableAutoConsumption(); + } + } + if (entry->store_status != STORE_PENDING) { debugs(4, 2, "Skipping error page due to store_status: " << entry->store_status); /*