From 6b9acd18a07987944459c285ac557c924e4d1d1c Mon Sep 17 00:00:00 2001 From: Henrik Nordstrom Date: Tue, 17 Jan 2012 20:17:58 +0100 Subject: [PATCH] - fix for gcc-4.7 - fix for bug #772483 running out of memory, mem_node growing out of bounds --- .gitignore | 1 + sources | 1 + squid-3.2-mem_node.patch | 449 +++++++++++++++++++++++++++++++++++++++ squid.spec | 19 +- 4 files changed, 468 insertions(+), 2 deletions(-) create mode 100644 squid-3.2-mem_node.patch diff --git a/.gitignore b/.gitignore index de6c0c4..fef8309 100644 --- a/.gitignore +++ b/.gitignore @@ -30,3 +30,4 @@ squid-3.1-10021.patch /squid-3.2.0.13.tar.bz2.asc /squid-3.2.0.14.tar.bz2 /squid-3.2.0.14.tar.bz2.asc +/squid-3.2-11480.patch diff --git a/sources b/sources index 0f1add7..5a0ada5 100644 --- a/sources +++ b/sources @@ -1,2 +1,3 @@ b71beaa91e8d0a9f89956ac3685478a8 squid-3.2.0.14.tar.bz2 d05ae51cde36421424214d4be7938847 squid-3.2.0.14.tar.bz2.asc +4b5c832c50e511e7af4a5b8ee1791d70 squid-3.2-11480.patch diff --git a/squid-3.2-mem_node.patch b/squid-3.2-mem_node.patch new file mode 100644 index 0000000..4c1d511 --- /dev/null +++ b/squid-3.2-mem_node.patch @@ -0,0 +1,449 @@ +diff -ru squid-3.2.0.14/src/MemObject.cc squid-3.2.0.14-mem_node/src/MemObject.cc +--- squid-3.2.0.14/src/MemObject.cc 2011-12-12 12:08:18.000000000 +0100 ++++ squid-3.2.0.14-mem_node/src/MemObject.cc 2012-01-17 20:11:54.756609310 +0100 +@@ -492,3 +492,9 @@ + } + + #endif ++ ++int64_t ++MemObject::availableForSwapOut() const ++{ ++ return endOffset() - swapout.queue_offset; ++} +diff -ru squid-3.2.0.14/src/MemObject.h squid-3.2.0.14-mem_node/src/MemObject.h +--- squid-3.2.0.14/src/MemObject.h 2011-12-12 12:08:18.000000000 +0100 ++++ squid-3.2.0.14-mem_node/src/MemObject.h 2012-01-17 20:11:54.757609437 +0100 +@@ -81,6 +81,7 @@ + */ + int64_t objectBytesOnDisk() const; + int64_t policyLowestOffsetToKeep(bool swap) const; ++ int64_t availableForSwapOut() const; ///< buffered bytes we have not swapped out yet + void trimSwappable(); + void trimUnSwappable(); + bool isContiguous() const; +diff -ru squid-3.2.0.14/src/store.cc squid-3.2.0.14-mem_node/src/store.cc +--- squid-3.2.0.14/src/store.cc 2011-12-12 12:08:18.000000000 +0100 ++++ squid-3.2.0.14-mem_node/src/store.cc 2012-01-17 20:11:54.761609945 +0100 +@@ -1890,95 +1890,8 @@ + return result; + } + +-bool +-StoreEntry::swapoutPossible() +-{ +- if (!Config.cacheSwap.n_configured) +- return false; +- +- /* should we swap something out to disk? */ +- debugs(20, 7, "storeSwapOut: " << url()); +- debugs(20, 7, "storeSwapOut: store_status = " << storeStatusStr[store_status]); +- +- assert(mem_obj); +- MemObject::SwapOut::Decision &decision = mem_obj->swapout.decision; +- +- // if we decided that swapout is not possible, do not repeat same checks +- if (decision == MemObject::SwapOut::swImpossible) { +- debugs(20, 3, "storeSwapOut: already rejected"); +- return false; +- } +- +- // this flag may change so we must check it even if we already said "yes" +- if (EBIT_TEST(flags, ENTRY_ABORTED)) { +- assert(EBIT_TEST(flags, RELEASE_REQUEST)); +- // StoreEntry::abort() already closed the swap out file, if any +- decision = MemObject::SwapOut::swImpossible; +- return false; +- } +- +- // if we decided that swapout is possible, do not repeat same checks +- if (decision == MemObject::SwapOut::swPossible) { +- debugs(20, 3, "storeSwapOut: already allowed"); +- return true; +- } +- +- // if we are swapping out already, do not repeat same checks +- if (swap_status != SWAPOUT_NONE) { +- debugs(20, 3, "storeSwapOut: already started"); +- decision = MemObject::SwapOut::swPossible; +- return true; +- } +- +- if (!checkCachable()) { +- debugs(20, 3, "storeSwapOut: not cachable"); +- decision = MemObject::SwapOut::swImpossible; +- return false; +- } +- +- if (EBIT_TEST(flags, ENTRY_SPECIAL)) { +- debugs(20, 3, "storeSwapOut: " << url() << " SPECIAL"); +- decision = MemObject::SwapOut::swImpossible; +- return false; +- } +- +- // check cache_dir max-size limit if all cache_dirs have it +- if (store_maxobjsize >= 0) { +- // TODO: add estimated store metadata size to be conservative +- +- // use guaranteed maximum if it is known +- const int64_t expectedEnd = mem_obj->expectedReplySize(); +- debugs(20, 7, "storeSwapOut: expectedEnd = " << expectedEnd); +- if (expectedEnd > store_maxobjsize) { +- debugs(20, 3, "storeSwapOut: will not fit: " << expectedEnd << +- " > " << store_maxobjsize); +- decision = MemObject::SwapOut::swImpossible; +- return false; // known to outgrow the limit eventually +- } +- +- // use current minimum (always known) +- const int64_t currentEnd = mem_obj->endOffset(); +- if (currentEnd > store_maxobjsize) { +- debugs(20, 3, "storeSwapOut: does not fit: " << currentEnd << +- " > " << store_maxobjsize); +- decision = MemObject::SwapOut::swImpossible; +- return false; // already does not fit and may only get bigger +- } +- +- // prevent default swPossible answer for yet unknown length +- if (expectedEnd < 0) { +- debugs(20, 3, "storeSwapOut: wait for more info: " << +- store_maxobjsize); +- return false; // may fit later, but will be rejected now +- } +- } +- +- decision = MemObject::SwapOut::swPossible; +- return true; +-} +- + void +-StoreEntry::trimMemory() ++StoreEntry::trimMemory(const bool preserveSwappable) + { + /* + * DPW 2007-05-09 +@@ -1988,7 +1901,7 @@ + if (mem_status == IN_MEMORY) + return; + +- if (!swapOutAble()) { ++ if (!preserveSwappable) { + if (mem_obj->policyLowestOffsetToKeep(0) == 0) { + /* Nothing to do */ + return; +Endast i squid-3.2.0.14-mem_node/src: store.cc.orig +diff -ru squid-3.2.0.14/src/store_client.cc squid-3.2.0.14-mem_node/src/store_client.cc +--- squid-3.2.0.14/src/store_client.cc 2011-12-12 12:08:18.000000000 +0100 ++++ squid-3.2.0.14-mem_node/src/store_client.cc 2012-01-17 20:11:54.768610834 +0100 +@@ -194,7 +194,7 @@ + if (getType() == STORE_DISK_CLIENT) + /* assert we'll be able to get the data we want */ + /* maybe we should open swapin_sio here */ +- assert(entry->swap_filen > -1 || entry->swapOutAble()); ++ assert(entry->swap_filen > -1 || entry->swappingOut()); + + #if STORE_CLIENT_LIST_DEBUG + +Endast i squid-3.2.0.14-mem_node/src: store_client.cc.orig +diff -ru squid-3.2.0.14/src/Store.h squid-3.2.0.14-mem_node/src/Store.h +--- squid-3.2.0.14/src/Store.h 2011-12-12 12:08:18.000000000 +0100 ++++ squid-3.2.0.14-mem_node/src/Store.h 2012-01-17 20:11:56.770865312 +0100 +@@ -92,8 +92,9 @@ + virtual char const *getSerialisedMetaData(); + void replaceHttpReply(HttpReply *, bool andStartWriting = true); + void startWriting(); ///< pack and write reply headers and, maybe, body +- virtual bool swapoutPossible(); +- virtual void trimMemory(); ++ /// whether we may start writing to disk (now or in the future) ++ virtual bool mayStartSwapOut(); ++ virtual void trimMemory(const bool preserveSwappable); + void abort(); + void unlink(); + void makePublic(); +@@ -108,7 +109,8 @@ + void purgeMem(); + void cacheInMemory(); ///< start or continue storing in memory cache + void swapOut(); +- bool swapOutAble() const; ++ /// whether we are in the process of writing this entry to disk ++ bool swappingOut() const { return swap_status == SWAPOUT_WRITING; } + void swapOutFileClose(int how); + const char *url() const; + int checkCachable(); +@@ -247,9 +249,9 @@ + store_client_t storeClientType() const {return STORE_MEM_CLIENT;} + + char const *getSerialisedMetaData(); +- bool swapoutPossible() {return false;} ++ bool mayStartSwapout() {return false;} + +- void trimMemory() {} ++ void trimMemory(const bool preserveSwappable) {} + + + static NullStoreEntry _instance; +diff -ru squid-3.2.0.14/src/store_swapout.cc squid-3.2.0.14-mem_node/src/store_swapout.cc +--- squid-3.2.0.14/src/store_swapout.cc 2011-12-12 12:08:18.000000000 +0100 ++++ squid-3.2.0.14-mem_node/src/store_swapout.cc 2012-01-17 20:11:54.771611216 +0100 +@@ -187,8 +187,20 @@ + if (!mem_obj) + return; + +- if (!swapoutPossible()) ++ // this flag may change so we must check even if we are swappingOut ++ if (EBIT_TEST(flags, ENTRY_ABORTED)) { ++ assert(EBIT_TEST(flags, RELEASE_REQUEST)); ++ // StoreEntry::abort() already closed the swap out file, if any ++ // no trimming: data producer must stop production if ENTRY_ABORTED + return; ++ } ++ ++ const bool weAreOrMayBeSwappingOut = swappingOut() || mayStartSwapOut(); ++ ++ trimMemory(weAreOrMayBeSwappingOut); ++ ++ if (!weAreOrMayBeSwappingOut) ++ return; // nothing else to do + + // Aborted entries have STORE_OK, but swapoutPossible rejects them. Thus, + // store_status == STORE_OK below means we got everything we wanted. +@@ -200,39 +212,10 @@ + if (mem_obj->swapout.sio != NULL) + debugs(20, 7, "storeSwapOut: storeOffset() = " << mem_obj->swapout.sio->offset() ); + +- // buffered bytes we have not swapped out yet +- int64_t swapout_maxsize = mem_obj->endOffset() - mem_obj->swapout.queue_offset; +- +- assert(swapout_maxsize >= 0); +- + int64_t const lowest_offset = mem_obj->lowestMemReaderOffset(); + + debugs(20, 7, HERE << "storeSwapOut: lowest_offset = " << lowest_offset); + +- // Check to see whether we're going to defer the swapout based upon size +- if (store_status != STORE_OK) { +- const int64_t expectedSize = mem_obj->expectedReplySize(); +- const int64_t maxKnownSize = expectedSize < 0 ? +- swapout_maxsize : expectedSize; +- debugs(20, 7, HERE << "storeSwapOut: maxKnownSize= " << maxKnownSize); +- +- if (maxKnownSize < store_maxobjsize) { +- /* +- * NOTE: the store_maxobjsize here is the max of optional +- * max-size values from 'cache_dir' lines. It is not the +- * same as 'maximum_object_size'. By default, store_maxobjsize +- * will be set to -1. However, I am worried that this +- * deferance may consume a lot of memory in some cases. +- * Should we add an option to limit this memory consumption? +- */ +- debugs(20, 5, "storeSwapOut: Deferring swapout start for " << +- (store_maxobjsize - maxKnownSize) << " bytes"); +- return; +- } +- } +- +-// TODO: it is better to trim as soon as we swap something out, not before +- trimMemory(); + #if SIZEOF_OFF_T <= 4 + + if (mem_obj->endOffset() > 0x7FFF0000) { +@@ -245,9 +228,9 @@ + if (swap_status == SWAPOUT_WRITING) + assert(mem_obj->inmem_lo <= mem_obj->objectBytesOnDisk() ); + +- if (!swapOutAble()) +- return; +- ++ // buffered bytes we have not swapped out yet ++ const int64_t swapout_maxsize = mem_obj->availableForSwapOut(); ++ assert(swapout_maxsize >= 0); + debugs(20, 7, "storeSwapOut: swapout_size = " << swapout_maxsize); + + if (swapout_maxsize == 0) { // swapped everything we got +@@ -373,19 +356,106 @@ + e->unlock(); + } + +-/* +- * Is this entry a candidate for writing to disk? +- */ + bool +-StoreEntry::swapOutAble() const ++StoreEntry::mayStartSwapOut() + { + dlink_node *node; + +- if (mem_obj->swapout.sio != NULL) ++ // must be checked in the caller ++ assert(!EBIT_TEST(flags, ENTRY_ABORTED)); ++ ++ if (!Config.cacheSwap.n_configured) ++ return false; ++ ++ assert(mem_obj); ++ MemObject::SwapOut::Decision &decision = mem_obj->swapout.decision; ++ ++ // if we decided that swapout is not possible, do not repeat same checks ++ if (decision == MemObject::SwapOut::swImpossible) { ++ debugs(20, 3, HERE << " already rejected"); ++ return false; ++ } ++ ++ // if we decided that swapout is possible, do not repeat same checks ++ if (decision == MemObject::SwapOut::swPossible) { ++ debugs(20, 3, HERE << "already allowed"); ++ return true; ++ } ++ ++ // if we are swapping out already, do not repeat same checks ++ if (swap_status != SWAPOUT_NONE) { ++ debugs(20, 3, HERE << " already started"); ++ decision = MemObject::SwapOut::swPossible; + return true; ++ } + +- if (mem_obj->inmem_lo > 0) ++ if (!checkCachable()) { ++ debugs(20, 3, HERE << "not cachable"); ++ decision = MemObject::SwapOut::swImpossible; + return false; ++ } ++ ++ if (EBIT_TEST(flags, ENTRY_SPECIAL)) { ++ debugs(20, 3, HERE << url() << " SPECIAL"); ++ decision = MemObject::SwapOut::swImpossible; ++ return false; ++ } ++ ++ // check cache_dir max-size limit if all cache_dirs have it ++ if (store_maxobjsize >= 0) { ++ // TODO: add estimated store metadata size to be conservative ++ ++ // use guaranteed maximum if it is known ++ const int64_t expectedEnd = mem_obj->expectedReplySize(); ++ debugs(20, 7, HERE << "expectedEnd = " << expectedEnd); ++ if (expectedEnd > store_maxobjsize) { ++ debugs(20, 3, HERE << "will not fit: " << expectedEnd << ++ " > " << store_maxobjsize); ++ decision = MemObject::SwapOut::swImpossible; ++ return false; // known to outgrow the limit eventually ++ } ++ ++ // use current minimum (always known) ++ const int64_t currentEnd = mem_obj->endOffset(); ++ if (currentEnd > store_maxobjsize) { ++ debugs(20, 3, HERE << "does not fit: " << currentEnd << ++ " > " << store_maxobjsize); ++ decision = MemObject::SwapOut::swImpossible; ++ return false; // already does not fit and may only get bigger ++ } ++ ++ // prevent default swPossible answer for yet unknown length ++ if (expectedEnd < 0) { ++ debugs(20, 3, HERE << "wait for more info: " << ++ store_maxobjsize); ++ return false; // may fit later, but will be rejected now ++ } ++ ++ if (store_status != STORE_OK) { ++ const int64_t maxKnownSize = expectedEnd < 0 ? ++ mem_obj->availableForSwapOut() : expectedEnd; ++ debugs(20, 7, HERE << "maxKnownSize= " << maxKnownSize); ++ if (maxKnownSize < store_maxobjsize) { ++ /* ++ * NOTE: the store_maxobjsize here is the max of optional ++ * max-size values from 'cache_dir' lines. It is not the ++ * same as 'maximum_object_size'. By default, store_maxobjsize ++ * will be set to -1. However, I am worried that this ++ * deferance may consume a lot of memory in some cases. ++ * Should we add an option to limit this memory consumption? ++ */ ++ debugs(20, 5, HERE << "Deferring swapout start for " << ++ (store_maxobjsize - maxKnownSize) << " bytes"); ++ return false; ++ } ++ } ++ } ++ ++ if (mem_obj->inmem_lo > 0) { ++ debugs(20, 3, "storeSwapOut: (inmem_lo > 0) imem_lo:" << mem_obj->inmem_lo); ++ decision = MemObject::SwapOut::swImpossible; ++ return false; ++ } + + /* + * If there are DISK clients, we must write to disk +@@ -394,21 +464,29 @@ + * therefore this should be an assert? + * RBC 20030708: We can use disk to avoid mem races, so this shouldn't be + * an assert. ++ * ++ * XXX: Not clear what "mem races" the above refers to, especially when ++ * dealing with non-cachable objects that cannot have multiple clients. ++ * ++ * XXX: If STORE_DISK_CLIENT needs SwapOut::swPossible, we have to check ++ * for that flag earlier, but forcing swapping may contradict max-size or ++ * other swapability restrictions. Change storeClientType() and/or its ++ * callers to take swap-in availability into account. + */ + for (node = mem_obj->clients.head; node; node = node->next) { +- if (((store_client *) node->data)->getType() == STORE_DISK_CLIENT) ++ if (((store_client *) node->data)->getType() == STORE_DISK_CLIENT) { ++ debugs(20, 3, HERE << "DISK client found"); ++ decision = MemObject::SwapOut::swPossible; + return true; ++ } + } + +- /* Don't pollute the disk with icons and other special entries */ +- if (EBIT_TEST(flags, ENTRY_SPECIAL)) +- return false; +- +- if (!EBIT_TEST(flags, ENTRY_CACHABLE)) +- return false; +- +- if (!mem_obj->isContiguous()) ++ if (!mem_obj->isContiguous()) { ++ debugs(20, 3, "storeSwapOut: not Contiguous"); ++ decision = MemObject::SwapOut::swImpossible; + return false; ++ } + ++ decision = MemObject::SwapOut::swPossible; + return true; + } +Endast i squid-3.2.0.14-mem_node/src: store_swapout.cc.orig +diff -ru squid-3.2.0.14/src/tests/stub_MemObject.cc squid-3.2.0.14-mem_node/src/tests/stub_MemObject.cc +--- squid-3.2.0.14/src/tests/stub_MemObject.cc 2011-12-12 12:08:18.000000000 +0100 ++++ squid-3.2.0.14-mem_node/src/tests/stub_MemObject.cc 2012-01-17 20:11:54.772611344 +0100 +@@ -47,6 +47,7 @@ + { + return data_hdr.endOffset(); + } ++int64_t MemObject::availableForSwapOut() const STUB_RETVAL(0) + + void + MemObject::trimSwappable() +Endast i squid-3.2.0.14-mem_node/src/tests: stub_MemObject.cc.orig +diff -ru squid-3.2.0.14/src/tests/stub_store.cc squid-3.2.0.14-mem_node/src/tests/stub_store.cc +--- squid-3.2.0.14/src/tests/stub_store.cc 2011-12-12 12:08:18.000000000 +0100 ++++ squid-3.2.0.14-mem_node/src/tests/stub_store.cc 2012-01-17 20:11:54.773611472 +0100 +@@ -26,8 +26,8 @@ + store_client_t StoreEntry::storeClientType() const STUB_RETVAL(STORE_NON_CLIENT) + char const *StoreEntry::getSerialisedMetaData() STUB_RETVAL(NULL) + void StoreEntry::replaceHttpReply(HttpReply *, bool andStartWriting) STUB +-bool StoreEntry::swapoutPossible() STUB_RETVAL(false) +-void StoreEntry::trimMemory() STUB ++bool StoreEntry::mayStartSwapOut() STUB_RETVAL(false) ++void StoreEntry::trimMemory(const bool preserveSwappable) STUB + void StoreEntry::abort() STUB + void StoreEntry::unlink() STUB + void StoreEntry::makePublic() STUB +@@ -41,7 +41,6 @@ + void StoreEntry::invokeHandlers() STUB + void StoreEntry::purgeMem() STUB + void StoreEntry::swapOut() STUB +-bool StoreEntry::swapOutAble() const STUB_RETVAL(false) + void StoreEntry::swapOutFileClose(int how) STUB + const char *StoreEntry::url() const STUB_RETVAL(NULL) + int StoreEntry::checkCachable() STUB_RETVAL(0) diff --git a/squid.spec b/squid.spec index 6abb8a2..e48bc32 100644 --- a/squid.spec +++ b/squid.spec @@ -24,7 +24,13 @@ Source98: perl-requires-squid.sh ## Source99: filter-requires-squid.sh # Upstream patches -#Patch001: http://www.squid-cache.org/Versions/v3/3.2/changesets/squid-3.2-XXXXX.patch +Patch001: http://www.squid-cache.org/Versions/v3/3.2/changesets/squid-3.2-11480.patch + +# Backported patches +# RHBZ #772483 +# http://www.squid-cache.org/Versions/v3/3.HEAD/changesets/squid-3-11969.patch +# http://www.squid-cache.org/Versions/v3/3.HEAD/changesets/squid-3-11970.patch +Patch101: squid-3.2-mem_node.patch # Local patches # Applying upstream patches first makes it less likely that local patches @@ -88,8 +94,13 @@ The squid-sysvinit contains SysV initscritps support. %prep %setup -q -#patch001 -p0 +# Upstream patches +%patch001 -p0 +# Backported patches +%patch101 -p1 -b .mem_node + +# Local patches %patch201 -p1 -b .config %patch202 -p1 -b .location %patch203 -p1 -b .perlpath @@ -303,6 +314,10 @@ fi /sbin/chkconfig --add squid >/dev/null 2>&1 || : %changelog +* Tue Jan 17 2012 Henrik Nordstrom - 7:3.2.0.14-4 +- fix for gcc-4.7 +- fix for bug #772483 running out of memory, mem_node growing out of bounds + * Thu Jan 05 2012 Henrik Nordstrom - 3.2.0.14-3 - rebuild for gcc-4.7.0