[squid: 1/2] - fix for gcc-4.7 - fix for bug #772483 running out of memory, mem_node growing out of bounds
Henrik Nordström
hno at fedoraproject.org
Tue Jan 17 19:26:41 UTC 2012
commit 6b9acd18a07987944459c285ac557c924e4d1d1c
Author: Henrik Nordstrom <henrik at henriknordstrom.net>
Date: Tue Jan 17 20:17:58 2012 +0100
- 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(-)
---
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 <henrik at henriknordstrom.net> - 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 <henrik at henriknordstrom.net> - 3.2.0.14-3
- rebuild for gcc-4.7.0
More information about the scm-commits
mailing list