[PATCH] Re: Seg fault: race conditions in mod_mem_cache.c
Bill Stoddard wrote: Not to knock over the apple cart or anything, but had a 15 second chat with Greg today and he had an idea that sounded good. His idea was to eliminate the cleanup bit entirely and fold the function it is providing into the refcount. With the code in HEAD right now, a cache object sitting in the cache hash table unreferenced by any worker threads has a refcount of 0. The change is to bump the refcount of a cache object by 1 to reflect that it is being 'referenced' by the hash table. This simplifies the code quite a bit (assuming I'm not overlooking a hole that will need plugging). Patch against HEAD on the way. Bill Please give this a try and see how it works... Index: mod_mem_cache.c === RCS file: /home/cvs/httpd-2.0/modules/experimental/mod_mem_cache.c,v retrieving revision 1.88.2.9 diff -u -r1.88.2.9 mod_mem_cache.c --- mod_mem_cache.c 26 Aug 2004 16:59:44 - 1.88.2.9 +++ mod_mem_cache.c 16 Sep 2004 03:48:02 - @@ -13,6 +13,14 @@ * limitations under the License. */ +/* + * obj->refcount should only be incremented under protection of the sconf->lock + * obj->refcount can be atomically decremented w/o protection of the sconf->lock + * obj->refcount is incremented by one when the object is in the cache and once + * per thread referencing the object. An object in the cache with no threads + * accessing it will have a refcount of 1. When refcount drops to 0, the object + * should be garbage collected. + */ #define CORE_PRIVATE #include "mod_cache.h" #include "cache_pqueue.h" @@ -142,24 +150,20 @@ return obj->key; } /** - * callback to free an entry - * There is way too much magic in this code. Right now, this callback - * is only called out of cache_insert() which is called under protection - * of the sconf->lock, which means that we do not (and should not) - * attempt to obtain the lock recursively. + * memcache_cache_free() + * memcacache_cache_free is a callback that is only invoked during by a thread + * running in cache_insert(). cache_insert() runs under protection + * of sconf->lock. By the time this function has been entered, the cache_object + * has been ejected from the cache. decrement the refcount and if the refcount drop + * to 0, cleanup the cache object. */ static void memcache_cache_free(void*a) { cache_object_t *obj = (cache_object_t *)a; -/* Cleanup the cache object. Object should be removed from the cache - * now. Increment the refcount before setting cleanup to avoid a race - * condition. A similar pattern is used in remove_url() +/* Decrement the refcount to account for the object being ejected + * from the cache. If the refcount is 0, free the object. */ -apr_atomic_inc(&obj->refcount); - -obj->cleanup = 1; - if (!apr_atomic_dec(&obj->refcount)) { cleanup_cache_object(obj); } @@ -267,31 +271,9 @@ { cache_object_t *obj = (cache_object_t *) arg; -/* If obj->complete is not set, the cache update failed and the - * object needs to be removed from the cache then cleaned up. - */ -if (!obj->complete) { -if (sconf->lock) { -apr_thread_mutex_lock(sconf->lock); -} -/* Remember, objects marked for cleanup are, by design, already - * removed from the cache. remove_url() could have already - * removed the object from the cache (and set obj->cleanup) - */ -if (!obj->cleanup) { -cache_remove(sconf->cache_cache, obj); -obj->cleanup = 1; -} -if (sconf->lock) { -apr_thread_mutex_unlock(sconf->lock); -} -} - -/* Cleanup the cache object */ +/* If the refcount drops to 0, cleanup the cache object */ if (!apr_atomic_dec(&obj->refcount)) { -if (obj->cleanup) { -cleanup_cache_object(obj); -} +cleanup_cache_object(obj); } return APR_SUCCESS; } @@ -312,10 +294,7 @@ } obj = cache_pop(co->cache_cache); while (obj) { -/* Iterate over the cache and clean up each entry */ -/* Free the object if the recount == 0 */ -apr_atomic_inc(&obj->refcount); -obj->cleanup = 1; +/* Iterate over the cache and clean up each unreferenced entry */ if (!apr_atomic_dec(&obj->refcount)) { cleanup_cache_object(obj); } @@ -415,7 +394,6 @@ apr_atomic_set(&obj->refcount, 1); mobj->total_refs = 1; obj->complete = 0; -obj->cleanup = 0; obj->vobj = mobj; /* Safe cast: We tested < sconf->max_cache_object_size above */ mobj->m_len = (apr_size_t)len; @@ -425,7 +403,7 @@ * Note: Perhaps we should wait to put the object in the * hash table when the object is complete? I add the object here to * avoid multiple threads attempting to cache the same content only - * to discover at the very end that only one of the
[STATUS] (httpd-2.1) Wed Sep 15 23:45:14 EDT 2004
APACHE 2.1 STATUS: -*-text-*- Last modified at [$Date: 2004/09/03 02:47:19 $] Release [NOTE that only Alpha/Beta releases occur in 2.1 development]: 2.1.0 : in development Please consult the following STATUS files for information on related projects: * srclib/apr/STATUS * srclib/apr-util/STATUS * docs/STATUS Contributors looking for a mission: * Just do an egrep on "TODO" or "XXX" in the source. * Review the "PatchAvailable" bugs in the bug database. Append a comment saying "Reviewed and tested". * Open bugs in the bug database. CURRENT RELEASE NOTES: * When the CVS->SVN is done, there's a bogus avendor branch that should be removed from most files. The branch was created 4/27/2004. It's safest (and easiest) for now just to leave it in there; the MAIN branch and the APACHE_2_0_BRANCH are untouched and unharmed. --jwoolley RELEASE SHOWSTOPPERS: * Handling of non-trailing / config by non-default handler is broken http://marc.theaimsgroup.com/?l=apache-httpd-dev&m=105451701628081&w=2 * the edge connection filter cannot be removed http://marc.theaimsgroup.com/?l=apache-httpd-dev&m=105366252619530&w=2 CURRENT VOTES: * Promote mod_cache from experimental to non-experimental status (keep issues noted below in EXPERIMENTAL MODULES as items to be addressed as a supported module). +1: jim, bnicholes -0: jerenkrantz -1: stoddard There are a couple of problems that need to be resolved before this module is moved out of experimental. 1) We need to at least review and comment on the RFC violations 2) Resolve issue of how to cache page fragements (or perhaps -if- we want to cache page fragements). Today, mod_cache/mod_mem_cache will cache #include 'virtual' requests (but not #include 'file' requests). This was accomplished by making CACHE_IN a CONTENT_SET-1 filter to force it to run before the SUBREQ_CORE filter. But now responses cannot be cached that include the effects of having been run through CONTENT_SET filters (mod_deflate, mod_expires, etc). We could rerun all the CONTENT_SET filters on the cached response, but this will not work in all cases. For example, mod_expires relies on installing the EXPIRATION filter during fixups. Contents served out of mod_cache (out of the quick_handler) bypass -all- the request line server hooks (Ryan really hated this. It is great for performance, but bad because of the complications listed above). jerenkrantz: There are a slew of RFC compliance bugs filed in Bugzilla for mod_cache (see 'RFC 2616 violations' below). I think fixing them is a pre-requisite before it isn't experimental. * httpd-std.conf and friends a) httpd-std.conf should be tailored by install (from src or binbuild) even if user has existing httpd.conf +1: trawick, slive, gregames, ianh, Ken, wrowe, jwoolley, jim, nd, erikabele wrowe - prefer httpd.default.conf to avoid ambiguity with cvs b) tailored httpd-std.conf should be copied by install to sysconfdir/examples -0: striker c) tailored httpd-std.conf should be installed to sysconfdir/examples or manualdir/exampleconf/ +1: slive, trawick, Ken, nd (prefer the latter), erikabele d) Installing a set of default config files when upgrading a server doesn't make ANY sense at all. +1: ianh - medium/big sites don't use 'standard config' anyway, as it usually needs major customizations -1: Ken, wrowe, jwoolley, jim, nd, erikabele wrowe - diff is wonderful when comparing old/new default configs, even for customized sites that ianh mentions jim - ... assuming that the default configs have been updated with the required inline docs to explain the changes * If the parent process dies, should the remaining child processes "gracefully" self-terminate. Or maybe we should make it a runtime option, or have a concept of 2 parent processes (one being a "hot spare"). See: Message-ID: <[EMAIL PROTECTED]> Self-destruct: Ken, Martin, Lars Not self-destruct: BrianP, Ian, Cliff, BillS Make it runtime configurable: Aaron, jim, Justin, wrowe, rederpj, nd /* The below was a concept on *how* to handle the problem */ Have 2 parents: +1: jim -1: Justin, wrowe, rederpj, nd +0: Lars, Martin (while standing by, could it do something useful?) * Make the worker MPM the default MPM for threaded Unix boxes. +1: Justin, Ian, Cliff, BillS, striker, wrowe, nd +0:
[STATUS] (httpd-2.0) Wed Sep 15 23:45:11 EDT 2004
APACHE 2.0 STATUS: -*-text-*- Last modified at [$Date: 2004/09/15 20:27:03 $] Release: 2.0.52 : in development 2.0.51 : released September 15, 2004 as GA. 2.0.50 : released June 30, 2004 as GA. 2.0.49 : released March 19, 2004 as GA. 2.0.48 : released October 29, 2003 as GA. 2.0.47 : released July 09, 2003 as GA. 2.0.46 : released May 28, 2003 as GA. 2.0.45 : released April 1, 2003 as GA. 2.0.44 : released January 20, 2003 as GA. 2.0.43 : released October 3, 2002 as GA. 2.0.42 : released September 24, 2002 as GA. 2.0.41 : rolled September 16, 2002. not released. 2.0.40 : released August 9, 2002 as GA. 2.0.39 : released June 17, 2002 as GA. 2.0.38 : rolled June 16, 2002. not released. 2.0.37 : rolled June 11, 2002. not released. 2.0.36 : released May 6, 2002 as GA. 2.0.35 : released April 5, 2002 as GA. 2.0.34 : tagged March 26, 2002. 2.0.33 : tagged March 6, 2002. not released. 2.0.32 : released Feburary 16, 2002 as beta. 2.0.31 : rolled Feburary 1, 2002. not released. 2.0.30 : tagged January 8, 2002. not rolled. 2.0.29 : tagged November 27, 2001. not rolled. 2.0.28 : released November 13, 2001 as beta. 2.0.27 : rolled November 6, 2001 2.0.26 : tagged October 16, 2001. not rolled. 2.0.25 : rolled August 29, 2001 2.0.24 : rolled August 18, 2001 2.0.23 : rolled August 9, 2001 2.0.22 : rolled July 29, 2001 2.0.21 : rolled July 20, 2001 2.0.20 : rolled July 8, 2001 2.0.19 : rolled June 27, 2001 2.0.18 : rolled May 18, 2001 2.0.17 : rolled April 17, 2001 2.0.16 : rolled April 4, 2001 2.0.15 : rolled March 21, 2001 2.0.14 : rolled March 7, 2001 2.0a9 : released December 12, 2000 2.0a8 : released November 20, 2000 2.0a7 : released October 8, 2000 2.0a6 : released August 18, 2000 2.0a5 : released August 4, 2000 2.0a4 : released June 7, 2000 2.0a3 : released April 28, 2000 2.0a2 : released March 31, 2000 2.0a1 : released March 10, 2000 Please consult the following STATUS files for information on related projects: * srclib/apr/STATUS * srclib/apr-util/STATUS * docs/STATUS Contributors looking for a mission: * Just do an egrep on "TODO" or "XXX" in the source. * Review the "PatchAvailable" bugs in the bug database. Append a comment saying "Reviewed and tested". * Open bugs in the bug database. RELEASE SHOWSTOPPERS: PATCHES TO BACKPORT FROM 2.1 [ please place file names and revisions from HEAD here, so it is easy to identify exactly what the proposed changes are! ] *) Use HTML 2.0 for error pages. PR 30732 modules/http/http_protocol.c: r1.483 +1: nd *) mod_mem_cache: Fixed race condition causing segfault because of memory being freed twice, or reused after being freed. Changed the atomic usage of refcount variable by combining it with a cleanup flag. modules/experimental/mod_mem_cache.c: r1.115 modules/experimental/mod_cache.h: r1.49 +1: clar *) mod_rewrite: Fix 0 bytes write into random memory position. PR 31036. (2.0 + 1.3) http://www.apache.org/~nd/dbmmap_1.3.patch http://www.apache.org/~nd/dbmmap_2.0.patch +1: nd *) mod_rewrite:Fix query string handling for proxied URLs. PR 14518. (2.0 + 1.3) modules/mappers/mod_rewrite.c: r1.259 +1: nd *) Don't link suexec against APR/etc libraries. http://cvs.apache.org/viewcvs.cgi/httpd-2.0/support/Makefile.in?r1=1.38&r2=1.39 +1: jorton, trawick, nd nd: what about the need of -static (dunno)? jorton: -static was needed only to make sure libapr etc were linked statically into suexec; they didn't work shared in a binary distribution because LD_LIBRARY_PATH etc are ignored for a setuid binary (that only matters for binary distributors where suexec gets relocated, since libtool puts an appropriate RPATH in the binary). Not linking suexec against libapr etc avoids the issue entirely (and avoids scary >1Mb suexec binaries) nd: oh well. *) mod_headers: Support {...}s tag for SSL variable lookup. http://www.apache.org/~jorton/mod_headers-2.0-ssl.diff +1: jorton, trawick nd: two comments: (1) is the use of APR_ASCII_* ebcdic-safe? I.e. shouldn't we use the native chars here and it will be converted later? (I'm not sure) jorton: I have no idea, let an EBCDIC-er complain if it breaks? trawick: seems that '\r' and '\n' are the better chars to check for; this is not raw data read from the network (or directly from SSL) but instead it is either protocol data that has already been converted to the native charset or it is other
[STATUS] (apache-1.3) Wed Sep 15 23:45:07 EDT 2004
APACHE 1.3 STATUS: -*-text-*- Last modified at [$Date: 2004/09/08 19:35:51 $] Release: 1.3.32-dev: In development. Jim proposes a release top of Sept. 1.3.31: Tagged May 7, 2004. Announced May 11, 2004. 1.3.30: Tagged April 9, 2004. Not released. 1.3.29: Tagged October 24, 2003. Announced Oct 29, 2003. 1.3.28: Tagged July 16, 2003. Announced ?? 1.3.27: Tagged September 30, 2002. Announced Oct 3, 2002. 1.3.26: Tagged June 18, 2002. 1.3.25: Tagged June 17, 2002. Not released. 1.3.24: Tagged Mar 21, 2002. Announced Mar 22, 2002. 1.3.23: Tagged Jan 21, 2002. 1.3.22: Tagged Oct 8, 2001. Announced Oct 12, 2001. 1.3.21: Not released. (Pulled for htdocs/manual config mismatch. t/r Oct 5, 2001) 1.3.20: Tagged and rolled May 15, 2001. Announced May 21, 2001. 1.3.19: Tagged and rolled Feb 26, 2001. Announced Mar 01, 2001. 1.3.18: Tagged and rolled Not released. (Pulled because of an incorrect unescaping fix. t/r Feb 19, 2001) 1.3.17: Tagged and rolled Jan 26, 2001. Announced Jan 29, 2001. 1.3.16: Not released. (Pulled because of vhosting bug. t/r Jan 20, 2001) 1.3.15: Not released. (Pulled due to CVS dumping core during the tagging when it reached src/os/win32/) 1.3.14: Tagged and Rolled Oct 10, 2000. Released/announced on the 13th. 1.3.13: Not released. (Pulled in the "first minutes" due to a Netware build bug) 1.3.12: Tagged and rolled Feb. 23, 2000. Released/announced on the 25th. 1.3.11: Tagged and rolled Jan. 19, 2000. Released/announced on the 21st. 1.3.10: Not released. (Pulled at "last minute" due to a build bug in the MPE port) 1.3.9: Tagged and rolled on Aug. 16, 1999. Released and announced on 19th. 1.3.8: Not released. 1.3.7: Not released. 1.3.6: Tagged and rolled on Mar. 22, 1999. Released and announced on 24th. 1.3.5: Not released. 1.3.4: Tagged and rolled on Jan. 9, 1999. Released on 11th, announced on 12th. 1.3.3: Tagged and rolled on Oct. 7, 1998. Released on 9th, announced on 10th. 1.3.2: Tagged and rolled on Sep. 21, 1998. Announced and released on 23rd. 1.3.1: Tagged and rolled on July 19, 1998. Announced and released. 1.3.0: Tagged and rolled on June 1, 1998. Announced and released on the 6th. 2.0 : Available for general use, see httpd-2.0 repository RELEASE SHOWSTOPPERS: PROPOSED PATCHES FOR THIS RELEASE: *) mod_rewrite: Fix 0 bytes write into random memory position. PR 31036. (2.0 + 1.3) http://www.apache.org/~nd/dbmmap_1.3.patch +1: nd *) mod_rewrite:Fix query string handling for proxied URLs. PR 14518. modules/mappers/mod_rewrite.c: r1.259 (2.x patch - need 1.3 version) +1: nd *) mod_log_config: Cleanup log_header_out function to allow multiple headers like Set-Cookie to be logged properly. PR 27787 modules/loggers/mod_log_config.c: r1.116 (2.x patch - need 1.3 version) jerenkrantz asks: Isn't this what apr_table_merge is for? nd replies: yep. But cookies won't be merged, because browsers don't support it. jerenkrantz: Couldn't we copy the table and merge the values somehow? This just seems like a lot of code to duplicate what we have already. *shrug* +1: nd, jerenkrantz RELEASE NON-SHOWSTOPPERS BUT WOULD BE REAL NICE TO WRAP THESE UP: * PR: 27023 Cookie could not delivered if the cookie made before proxy module. * isn't ap_die() broken with recognizing recursive errors Message-Id: <[EMAIL PROTECTED]> +1: jeff, jim * Current vote on 3 PRs for inclusion: Bugz #17877 (passing chunked encoding thru proxy) (still checking if RFC compliant... vote is on the correctness of the patch code only). +1: jim, chuck, minfrin Bugz #9181 (Unable to set headers on non-2XX responses) +1: Martin, Jim Gnats #10246 (Add ProxyConnAllow directive) +0: Martin (or rather -.5, see dev@ Message <[EMAIL PROTECTED]>) * htpasswd.c and htdigest.c use tmpnam()... consider using mkstemp() when available. Message-ID: <[EMAIL PROTECTED]> Status: * Dean's "unescaping hell" (unescaping the various URI components at the right time and place, esp. unescaping the host name). Message-ID: <[EMAIL PROTECTED]> Status: * Martin observed a core dump because a ipaddr_chain struct contains a NULL-"server" pointer when being dereferenced by invoking "httpd -S". Message-ID: <[EMAIL PROTECTED]> Status: Workaround enabled. Clean solution can come after 1.3.19 * long pathnames with many components and no AllowOverride None Workaround is to define with AllowOverride None, which is something all sites should do in any ca
Re: [PATCH2] Re: Seg fault: race conditions in mod_mem_cache.c
Not to knock over the apple cart or anything, but had a 15 second chat with Greg today and he had an idea that sounded good. His idea was to eliminate the cleanup bit entirely and fold the function it is providing into the refcount. With the code in HEAD right now, a cache object sitting in the cache hash table unreferenced by any worker threads has a refcount of 0. The change is to bump the refcount of a cache object by 1 to reflect that it is being 'referenced' by the hash table. This simplifies the code quite a bit (assuming I'm not overlooking a hole that will need plugging). Patch against HEAD on the way. Bill
Re: [PATCH2] Re: Seg fault: race conditions in mod_mem_cache.c
>>> [EMAIL PROTECTED] 09/15/04 12:41 PM >>> >I'm still trying to learn the complete design though. I see that >memcache_cache_free() can return with OBJECT_CLEANUP_BIT set, and that >decrement_refcount() will react to it and free the memory if it is registered as >a cleanup. Will decrement_refcount() always be registered? Just wondering if >there are any code paths where we might leak memory. An element will either be created or opened, both cases are registering decrement_refcount() as the cleanup function to be called when the thread is done. To remove the 2 noted possible race situations, in addition to another possible one in store_body(), we could add the following 2 recursive functions, that have to be called under the protection of the lock (it is now for all 3 cases of atomic_set32()): static void memcache_clear_cleanup_bit(cache_object_t *obj){ apr_uint32_t tmp_refcount = obj->refcount; if(tmp_refcount != apr_atomic_cas32(&obj->refcount, obj->refcount & ~OBJECT_CLEANUP_BIT, tmp_refcount)) { memcache_clear_cleanup_bit(obj); }}static void memcache_set_cleanup_bit(cache_object_t *obj){ apr_uint32_t tmp_refcount = obj->refcount; if(tmp_refcount != apr_atomic_cas32(&obj->refcount, tmp_refcount | OBJECT_CLEANUP_BIT, tmp_refcount)) { memcache_set_cleanup_bit(obj); }} One only additional thing is if I could call the set_cleanup_bit() within memcache_cache_free(), it would centralize all the bit setting into the 2 previous functions. Thanks for your time on that case. JJ
Re: [PATCH] fix child reclaim timing
On Wed, Sep 15, 2004 at 01:48:11PM -0400, Jeff Trawick wrote: > Here's a patch that does something like I mentioned above, though it > bails out a bit sooner (9 or so seconds). The timing of the > interesting actions in this patch can be tweaked in a much simpler > manner than the old 1.3 code allows. Looks just as good to me, +1 > - */ > +while (1) { > apr_sleep(waittime); ... > -if (!not_dead_yet) { > -/* nothing left to wait for */ > +if (!not_dead_yet || > +action_table[cur_action].action == GIVEUP) { > +/* nothing left to wait for, or we gave up */ > break; > } > } ...is crying out to be a do/while :)
Re: [PATCH2] Re: Seg fault: race conditions in mod_mem_cache.c
Jean-Jacques Clar wrote: What about calling memcache_cache_free() instead of the apr_atomic_set(). The refcount must be more than 1, we hold the mutex. It should work just fine.. What do you think? That looks better as far as not loosing an update from an unlocked thread. I'm still trying to learn the complete design though. I see that memcache_cache_free() can return with OBJECT_CLEANUP_BIT set, and that decrement_refcount() will react to it and free the memory if it is registered as a cleanup. Will decrement_refcount() always be registered? Just wondering if there are any code paths where we might leak memory. There's a similar race in remove_entity() using apr_atomic_set() to turn on the cleanup bit: @@ -536,10 +529,10 @@ * an object marked for cleanup is by design not in the * hash table. */ -if (!obj->cleanup) { -cache_remove(sconf->cache_cache, obj); -obj->cleanup = 1; -ap_log_error(APLOG_MARK, APLOG_INFO, 0, NULL, "gcing a cache entry"); + if (!(apr_atomic_read(&obj->refcount) & OBJECT_CLEANUP_BIT)) { + cache_remove(sconf->cache_cache, obj); + apr_atomic_set(&obj->refcount, obj->refcount | OBJECT_CLEANUP_BIT); + ap_log_error(APLOG_MARK, APLOG_INFO, 0, NULL, "gcing a cache entry"); } Greg
Re: [PATCH] fix child reclaim timing
On Fri, 13 Aug 2004 10:27:11 -0400, Jeff Trawick <[EMAIL PROTECTED]> wrote: > Here is my take on what is wrong with current code: > > 1) It starts complaining a bit too soon. Some third-party modules > have rather complicated child exit strategies. Whether or not that is > good or bad (bad ;) ), it results in disturbing messages that wouldn't > have appeared if we were a little more patient (2-3 seconds). Also, I > suspect that the use of threaded MPM affects how quickly the children > are exiting now on Unix. > > 2) It should never stop checking for exited processes less often than > 1-2 seconds, even if it doesn't complain to error log that often. > Like you say, current code can wait a VERY long time for child > processes to exit. In practice, I see that it can wait a VERY long > time even after the last child has exited. > > I'll agree that it should never wait so long, though I think around 15 > or so seconds total is reasonable. Exiting before children are gone > doesn't let Apache start up any more quickly; it just prevents > potentially-useful information about timing from getting logged to the > error log. > > --/-- > > I wouldn't complain to error log at all until it has been 2 seconds, > and then I'd still wait around for 10-15 more. But it has to check > every second so it finds out soon after all children have exited and > doesn't sleep needlessly. > Here's a patch that does something like I mentioned above, though it bails out a bit sooner (9 or so seconds). The timing of the interesting actions in this patch can be tweaked in a much simpler manner than the old 1.3 code allows. patch Description: Binary data
Floating 1.3.32
Please review STATUS for Apache 1.3.32-dev HEAD. I'd like to release soon. There are 3 proposed patches for inclusion noted, 2 of which we need 1.3 patch files for.
Re: [VOTE] Apache HTTP Server 2.0.51
At 10:49 AM 9/15/2004, you wrote: >Is there an ETA on the Win32 source bundle? About one hour. Bill
Re: [PATCH2] Re: Seg fault: race conditions in mod_mem_cache.c
What about calling memcache_cache_free() instead of the apr_atomic_set(). The refcount must be more than 1, we hold the mutex. It should work just fine.. What do you think? JJ>>> [EMAIL PROTECTED] 09/15/04 8:26 AM >>> Jean-Jacques Clar wrote:> Should I then go ahead and commit my patch to the 2.1 tree?This section from decrement_refcount() makes me nervous:- if (!obj->cleanup) {+ if (!(apr_atomic_read(&obj->refcount) & OBJECT_CLEANUP_BIT)) { cache_remove(sconf->cache_cache, obj);- obj->cleanup = 1;+ apr_atomic_set(&obj->refcount, obj->refcount | OBJECT_CLEANUP_BIT); } if (sconf->lock) { apr_thread_mutex_unlock(sconf->lock);@@ -288,10 +288,8 @@ } /* Cleanup the cache object */- if (!apr_atomic_dec(&obj->refcount)) {- if (obj->cleanup) {- cleanup_cache_object(obj);- }+ if(apr_atomic_dec(&obj->refcount) == OBJECT_CLEANUP_BIT) {+ cleanup_cache_object(obj); }It's the use of apr_atomic_set(). That line will take a snapshot of the value of obj->refcount at some point in time, OR on the OBJECT_CLEANUP_BIT, then unconditionally store the result back into obj->refcount.But there could be an unlocked caller trying to simultaneously decrement the refcount on another thread, right? I say that because of the use of apr_atomic_dec() a few lines later. If so, the apr_atomic_set could loose the decrement from the other thread. It would be better to use apr_atomic_cas() to set the cleanup bit because we can retry if the refcount changes.Greg
Antw: [VOTE] Apache HTTP Server 2.0.51
Builds and runs OK on WIN32 as well. André >>> [EMAIL PROTECTED] 15.09.2004 15:46:45 >>> Hi, I've put the tarballs for 2.0.51 up at http://httpd.apache.org/dev/dist/. Please test and vote, Sander
Re: [VOTE] Apache HTTP Server 2.0.51
Is there an ETA on the Win32 source bundle? Thanks. -- Jess Holle Sander Striker wrote: From: "Joshua Slive" <[EMAIL PROTECTED]> Sent: Wednesday, September 15, 2004 4:33 PM On Wed, 15 Sep 2004, Thom May wrote: * Sander Striker ([EMAIL PROTECTED]) wrote : Hi, I've put the tarballs for 2.0.51 up at http://httpd.apache.org/dev/dist/. Please test and vote, +1 on x86_64 linux. (Worker and prefork. perchild built but not tested) +1 Running on ajax.apache.org (www.eu.apache.org), which is ia64 Linux. Joshua. Thanks. I've moved the tarballs to the distribution area. Sander
Re: [PATCH2] Re: Seg fault: race conditions in mod_mem_cache.c
Jean-Jacques Clar wrote: Should I then go ahead and commit my patch to the 2.1 tree? This section from decrement_refcount() makes me nervous: -if (!obj->cleanup) { +if (!(apr_atomic_read(&obj->refcount) & OBJECT_CLEANUP_BIT)) { cache_remove(sconf->cache_cache, obj); -obj->cleanup = 1; +apr_atomic_set(&obj->refcount, obj->refcount | OBJECT_CLEANUP_BIT); } if (sconf->lock) { apr_thread_mutex_unlock(sconf->lock); @@ -288,10 +288,8 @@ } /* Cleanup the cache object */ -if (!apr_atomic_dec(&obj->refcount)) { -if (obj->cleanup) { -cleanup_cache_object(obj); -} +if(apr_atomic_dec(&obj->refcount) == OBJECT_CLEANUP_BIT) { +cleanup_cache_object(obj); } It's the use of apr_atomic_set(). That line will take a snapshot of the value of obj->refcount at some point in time, OR on the OBJECT_CLEANUP_BIT, then unconditionally store the result back into obj->refcount. But there could be an unlocked caller trying to simultaneously decrement the refcount on another thread, right? I say that because of the use of apr_atomic_dec() a few lines later. If so, the apr_atomic_set could loose the decrement from the other thread. It would be better to use apr_atomic_cas() to set the cleanup bit because we can retry if the refcount changes. Greg
Re: [VOTE] Apache HTTP Server 2.0.51
Sander Striker wrote: I've put the tarballs for 2.0.51 up at http://httpd.apache.org/dev/dist/. Please test and vote, Tested the RPM build under RHEL3, and it worked fine: +1. Regards, Graham --
Re: [VOTE] Apache HTTP Server 2.0.51
On Wed, 15 Sep 2004, Sander Striker wrote: From: "Sander Striker" <[EMAIL PROTECTED]> Sent: Wednesday, September 15, 2004 4:49 PM Thanks. I've moved the tarballs to the distribution area. Could someone please take care of the httpd.apache.org site? I'm in a bit of a bind currently (for at least another 1-2 hours). Done. Joshua.
Re: [VOTE] Apache HTTP Server 2.0.51
> From: "Sander Striker" <[EMAIL PROTECTED]> > Sent: Wednesday, September 15, 2004 4:49 PM > Thanks. I've moved the tarballs to the distribution area. Could someone please take care of the httpd.apache.org site? I'm in a bit of a bind currently (for at least another 1-2 hours). Sander
Re: [VOTE] Apache HTTP Server 2.0.51
From: "Joshua Slive" <[EMAIL PROTECTED]> Sent: Wednesday, September 15, 2004 4:33 PM > > On Wed, 15 Sep 2004, Thom May wrote: > > > * Sander Striker ([EMAIL PROTECTED]) wrote : > >> Hi, > >> > >> I've put the tarballs for 2.0.51 up at http://httpd.apache.org/dev/dist/. > >> Please test and vote, > >> > > +1 on x86_64 linux. (Worker and prefork. perchild built but not tested) > > +1 > > Running on ajax.apache.org (www.eu.apache.org), which is ia64 Linux. > > Joshua. Thanks. I've moved the tarballs to the distribution area. Sander
Re: [VOTE] Apache HTTP Server 2.0.51
On Wed, 15 Sep 2004, Thom May wrote: * Sander Striker ([EMAIL PROTECTED]) wrote : Hi, I've put the tarballs for 2.0.51 up at http://httpd.apache.org/dev/dist/. Please test and vote, +1 on x86_64 linux. (Worker and prefork. perchild built but not tested) +1 Running on ajax.apache.org (www.eu.apache.org), which is ia64 Linux. Joshua.
Re: [VOTE] Apache HTTP Server 2.0.51
On Wed, Sep 15, 2004 at 03:46:45PM +0200, Sander Striker wrote: > I've put the tarballs for 2.0.51 up at http://httpd.apache.org/dev/dist/. > Please test and vote, +1, build+httpd-test good on the usual Linuxes here, confirmed security fixes for mod_dav_fs, ap_resolve_env and apr_uri_parse on x86_64. joe
Re: [VOTE] Apache HTTP Server 2.0.51
* Sander Striker ([EMAIL PROTECTED]) wrote : > Hi, > > I've put the tarballs for 2.0.51 up at http://httpd.apache.org/dev/dist/. > Please test and vote, > +1 on x86_64 linux. (Worker and prefork. perchild built but not tested) -Thom -- That sounds like a lot of work... Can we out source? The Revolution will not be outsourced! (Slick/Monique - Sinfest)
Antw: httpd 2.0.51 release schedule?
Hello, as there is a security problem in webdav of 2.0.50, I think a 2.0.51 release will happen soon. http://www.heise.de/security/news/meldung/51086 http://www.securitytracker.com/alerts/2004/Sep/1011248.html André >>> [EMAIL PROTECTED] 15.09.2004 10:10:15 >>> Hi all, I am just about finishing a Flash MX 2004 book for O'Reilly Germany. As one of its topics is Flash and Server-Side Programming, the book's CD-ROM is going to contain Apache. The master CD has to be ready on Friday, so my question: Will the final release of httpd 2.0.51 happen until then, or should I consider including 2.0.50 instead? Regards, Sascha Btw: 2.0.51-rc2 builds and installs fine on SuSE Linux 9.0 (x86).
[VOTE] Apache HTTP Server 2.0.51
Hi, I've put the tarballs for 2.0.51 up at http://httpd.apache.org/dev/dist/. Please test and vote, Sander
httpd 2.0.51 release schedule?
Hi all, I am just about finishing a Flash MX 2004 book for O'Reilly Germany. As one of its topics is Flash and Server-Side Programming, the book's CD-ROM is going to contain Apache. The master CD has to be ready on Friday, so my question: Will the final release of httpd 2.0.51 happen until then, or should I consider including 2.0.50 instead? Regards, Sascha Btw: 2.0.51-rc2 builds and installs fine on SuSE Linux 9.0 (x86).