Hello,
The attached patch fixes a memory leak related to parsing
configurable SSL error details.
Trunk r11496 "Configurable SSL error details messages" correctly
disabled collection of HTTP statistics for non-HTTP header fields, such
as configurable SSL error details. However, it also incorrectly disabled
deletion of those non-HTTP header fields.
Configurable SSL error details are only created during [re]configuration
time, so the leak went unnoticed since 2011-06-17. However, when
annotations were added to non-HTTP header fields (r12413), the old bug
resulted in a major leak for configurations that annotate transactions.
Fortunately, that major leak was removed by r12779 "helperNotes to
NotePairs" polishing, which stopped using HttpHeader for annotations.
However, any code based on trunk between r12413 and r12779 needs this
fix (or needs to be resynced with the current trunk).
HTH,
Alex.
Bug: Leaking configurable SSL error details.
Trunk r11496 "Configurable SSL error details messages" correctly
disabled collection of HTTP statistics for non-HTTP header fields,
such as configurable SSL error details. However, it also incorrectly
disabled deletion of those non-HTTP header fields.
Configurable SSL error details are only created during [re]configuration
time, so the leak went unnoticed since 2011-06-17. However, when annotations
were added to non-HTTP header fields (r12413), the old bug resulted in a major
leak for configurations that annotate transactions.
Fortunately, that major leak was removed by r12779 "helperNotes to NotePairs"
polishing, which stopped using HttpHeader for annotations.
=== modified file 'src/HttpHeader.cc'
--- src/HttpHeader.cc 2012-11-26 18:54:43 +0000
+++ src/HttpHeader.cc 2013-05-10 21:50:56 +0000
@@ -421,71 +421,71 @@
// we do not really care, but the caller probably does
assert(owner == other.owner);
clean();
update(&other, NULL); // will update the mask as well
len = other.len;
}
return *this;
}
void
HttpHeader::clean()
{
HttpHeaderPos pos = HttpHeaderInitPos;
HttpHeaderEntry *e;
assert(owner > hoNone && owner < hoEnd);
debugs(55, 7, "cleaning hdr: " << this << " owner: " << owner);
PROF_start(HttpHeaderClean);
+ if (owner <= hoReply) {
/*
* An unfortunate bug. The entries array is initialized
* such that count is set to zero. httpHeaderClean() seems to
* be called both when 'hdr' is created, and destroyed. Thus,
* we accumulate a large number of zero counts for 'hdr' before
* it is ever used. Can't think of a good way to fix it, except
* adding a state variable that indicates whether or not 'hdr'
* has been used. As a hack, just never count zero-sized header
* arrays.
*/
-
- if (owner <= hoReply) {
if (0 != entries.count)
HttpHeaderStats[owner].hdrUCountDistr.count(entries.count);
++ HttpHeaderStats[owner].destroyedCount;
HttpHeaderStats[owner].busyDestroyedCount += entries.count > 0;
+ } // if (owner <= hoReply)
while ((e = getEntry(&pos))) {
/* tmp hack to try to avoid coredumps */
if (e->id < 0 || e->id >= HDR_ENUM_END) {
debugs(55, DBG_CRITICAL, "HttpHeader::clean BUG: entry[" << pos << "] is invalid (" << e->id << "). Ignored.");
} else {
+ if (owner <= hoReply)
HttpHeaderStats[owner].fieldTypeDistr.count(e->id);
/* yes, this deletion leaves us in an inconsistent state */
delete e;
}
}
- } // if (owner <= hoReply)
entries.clean();
httpHeaderMaskInit(&mask, 0);
len = 0;
PROF_stop(HttpHeaderClean);
}
/* append entries (also see httpHeaderUpdate) */
void
HttpHeader::append(const HttpHeader * src)
{
const HttpHeaderEntry *e;
HttpHeaderPos pos = HttpHeaderInitPos;
assert(src);
assert(src != this);
debugs(55, 7, "appending hdr: " << this << " += " << src);
while ((e = src->getEntry(&pos))) {
addEntry(e->clone());
}
}
