From fcb4beb31eb61497fbb38d3868ba9ebd0f5ce3ca Mon Sep 17 00:00:00 2001 From: Susan Hinrichs Date: Fri, 20 Nov 2020 01:52:32 +0000 Subject: [PATCH 1/3] Adjust flags to ensure tunnel producer is cleaned up --- proxy/http/HttpTunnel.cc | 4 ++-- proxy/http/HttpTunnel.h | 5 ++++- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/proxy/http/HttpTunnel.cc b/proxy/http/HttpTunnel.cc index e87cad1c62a..17ffab74aff 100644 --- a/proxy/http/HttpTunnel.cc +++ b/proxy/http/HttpTunnel.cc @@ -1332,9 +1332,9 @@ HttpTunnel::consumer_handler(int event, HttpTunnelConsumer *c) if (c->producer && c->producer->handler_state == 0) { if (event == VC_EVENT_WRITE_COMPLETE) { c->producer->handler_state = HTTP_SM_POST_SUCCESS; - // If the consumer completed, presumably the producer successfully read and is done + // If the consumer completed, presumably the producer successfully read + // Not marking the producer not alive, so the regular cleanup logic will be triggered c->producer->read_success = true; - c->producer->alive = false; } else if (c->vc_type == HT_HTTP_SERVER) { c->producer->handler_state = HTTP_SM_POST_UA_FAIL; } else if (c->vc_type == HT_HTTP_CLIENT) { diff --git a/proxy/http/HttpTunnel.h b/proxy/http/HttpTunnel.h index c6aa9fcb354..de5f2628558 100644 --- a/proxy/http/HttpTunnel.h +++ b/proxy/http/HttpTunnel.h @@ -396,8 +396,11 @@ HttpTunnel::is_tunnel_alive() const { bool tunnel_alive = false; + // Really a tunnel is only alive as long as there are consumers + // to consume. Or if there are no consumer (e.g. tunneling into a buffer) + // an alive producer may count for (const auto &producer : producers) { - if (producer.alive == true) { + if (producer.alive == true && producer.consumer_list.head == nullptr) { tunnel_alive = true; break; } From 1059720a3f40262d32ce9f463c3fedee5b304242 Mon Sep 17 00:00:00 2001 From: Susan Hinrichs Date: Mon, 30 Nov 2020 19:26:18 +0000 Subject: [PATCH 2/3] Another approach to avoid debug assert --- proxy/http/HttpTunnel.cc | 12 +++++++++++- proxy/http/HttpTunnel.h | 5 +---- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/proxy/http/HttpTunnel.cc b/proxy/http/HttpTunnel.cc index 17ffab74aff..33b03ad3526 100644 --- a/proxy/http/HttpTunnel.cc +++ b/proxy/http/HttpTunnel.cc @@ -1333,8 +1333,18 @@ HttpTunnel::consumer_handler(int event, HttpTunnelConsumer *c) if (event == VC_EVENT_WRITE_COMPLETE) { c->producer->handler_state = HTTP_SM_POST_SUCCESS; // If the consumer completed, presumably the producer successfully read - // Not marking the producer not alive, so the regular cleanup logic will be triggered c->producer->read_success = true; + // Go ahead and clean up the producer side + p->alive = false; + if (p->read_vio) { + p->bytes_read = p->read_vio->ndone; + } else { + p->bytes_read = 0; + } + if (p->vc != HTTP_TUNNEL_STATIC_PRODUCER) { + // Clear any outstanding reads + p->vc->do_io_read(nullptr, 0, nullptr); + } } else if (c->vc_type == HT_HTTP_SERVER) { c->producer->handler_state = HTTP_SM_POST_UA_FAIL; } else if (c->vc_type == HT_HTTP_CLIENT) { diff --git a/proxy/http/HttpTunnel.h b/proxy/http/HttpTunnel.h index de5f2628558..c6aa9fcb354 100644 --- a/proxy/http/HttpTunnel.h +++ b/proxy/http/HttpTunnel.h @@ -396,11 +396,8 @@ HttpTunnel::is_tunnel_alive() const { bool tunnel_alive = false; - // Really a tunnel is only alive as long as there are consumers - // to consume. Or if there are no consumer (e.g. tunneling into a buffer) - // an alive producer may count for (const auto &producer : producers) { - if (producer.alive == true && producer.consumer_list.head == nullptr) { + if (producer.alive == true) { tunnel_alive = true; break; } From f6b49bb6703e99e4efc639bbebf63acede20e393 Mon Sep 17 00:00:00 2001 From: Susan Hinrichs Date: Wed, 2 Dec 2020 22:03:56 +0000 Subject: [PATCH 3/3] Fix issue revealed by websocket test --- proxy/http/HttpTunnel.cc | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/proxy/http/HttpTunnel.cc b/proxy/http/HttpTunnel.cc index 33b03ad3526..07efb08ffac 100644 --- a/proxy/http/HttpTunnel.cc +++ b/proxy/http/HttpTunnel.cc @@ -1335,15 +1335,17 @@ HttpTunnel::consumer_handler(int event, HttpTunnelConsumer *c) // If the consumer completed, presumably the producer successfully read c->producer->read_success = true; // Go ahead and clean up the producer side - p->alive = false; - if (p->read_vio) { - p->bytes_read = p->read_vio->ndone; - } else { - p->bytes_read = 0; - } - if (p->vc != HTTP_TUNNEL_STATIC_PRODUCER) { - // Clear any outstanding reads - p->vc->do_io_read(nullptr, 0, nullptr); + if (p->alive) { + p->alive = false; + if (p->read_vio) { + p->bytes_read = p->read_vio->ndone; + } else { + p->bytes_read = 0; + } + if (p->vc != HTTP_TUNNEL_STATIC_PRODUCER) { + // Clear any outstanding reads + p->vc->do_io_read(nullptr, 0, nullptr); + } } } else if (c->vc_type == HT_HTTP_SERVER) { c->producer->handler_state = HTTP_SM_POST_UA_FAIL;