Re: cvs commit: apr/lib apr_pools.c
On Wed, Jan 03, 2001 at 12:53:07AM -, [EMAIL PROTECTED] wrote: > rbb 01/01/02 16:53:07 > > Modified:lib apr_pools.c > Log: > We should not be assigning permanent_pool inside of make_sub_pool. This > runs the risk of overwriting permanent_pool, which would be VERY bad. By > moving this back to apr_init_alloc, we know that it should only be called > by apr_initialize. This does mean that we can NEVER call apr_initialize > twice in the same program, but that should already be taken care of. apr_initialize() has a counter to prevent double-initializations from happening. This is quite a safe change. Cheers, -g -- Greg Stein, http://www.lyra.org/
Re: cvs commit: apr/lib apr_pools.c
[EMAIL PROTECTED] writes:
> rbb 01/01/02 16:53:07
>
> Modified:lib apr_pools.c
> Index: apr_pools.c
> ===
> RCS file: /home/cvs/apr/lib/apr_pools.c,v
> retrieving revision 1.76
> retrieving revision 1.77
> diff -u -r1.76 -r1.77
> --- apr_pools.c 2001/01/02 01:33:05 1.76
> +++ apr_pools.c 2001/01/03 00:53:05 1.77
> @@ -493,9 +493,6 @@
> }
> p->sub_pools = new_pool;
>}
> -else {
> -permanent_pool = new_pool;
> -}
>
>#if APR_HAS_THREADS
>if (alloc_mutex) {
> @@ -691,6 +688,7 @@
>return status;
>}
>#endif
> +permanent_pool = apr_make_sub_pool(pglobal);
How the [EMAIL PROTECTED] does this compile for you? Surely it does (yeah,
right)
as you had another commit to apr_pools.c after this one.
apr_pools.c: In function `apr_init_alloc':
apr_pools.c:691: `pglobal' undeclared (first use in this function)
apr_pools.c:691: (Each undeclared identifier is reported only once
apr_pools.c:691: for each function it appears in.)
apr_pools.c:691: too few arguments to function `apr_make_sub_pool'
--
Jeff Trawick | [EMAIL PROTECTED] | PGP public key at web site:
http://www.geocities.com/SiliconValley/Park/9289/
Born in Roswell... married an alien...
Re: cvs commit: apr/lib apr_pools.c
On Tue, 2 Jan 2001, Greg Stein wrote: > On Wed, Jan 03, 2001 at 12:53:07AM -, [EMAIL PROTECTED] wrote: > > rbb 01/01/02 16:53:07 > > > > Modified:lib apr_pools.c > > Log: > > We should not be assigning permanent_pool inside of make_sub_pool. This > > runs the risk of overwriting permanent_pool, which would be VERY bad. By > > moving this back to apr_init_alloc, we know that it should only be called > > by apr_initialize. This does mean that we can NEVER call apr_initialize > > twice in the same program, but that should already be taken care of. > > apr_initialize() has a counter to prevent double-initializations from > happening. This is quite a safe change. That's what I meant by "but that should already be taken care of." I guess the previous statement should have been we can't "execute the meat of apr_initialize twice in the same program". Oh well, clarity and I have never been good friends. :-) Ryan ___ Ryan Bloom [EMAIL PROTECTED] 406 29th St. San Francisco, CA 94131 ---
Re: cvs commit: apr/lib apr_pools.c
> > p->sub_pools = new_pool;
> >}
> > -else {
> > -permanent_pool = new_pool;
> > -}
> >
> >#if APR_HAS_THREADS
> >if (alloc_mutex) {
> > @@ -691,6 +688,7 @@
> >return status;
> >}
> >#endif
> > +permanent_pool = apr_make_sub_pool(pglobal);
>
> How the [EMAIL PROTECTED] does this compile for you? Surely it does (yeah,
> right)
> as you had another commit to apr_pools.c after this one.
>
> apr_pools.c: In function `apr_init_alloc':
> apr_pools.c:691: `pglobal' undeclared (first use in this function)
> apr_pools.c:691: (Each undeclared identifier is reported only once
> apr_pools.c:691: for each function it appears in.)
> apr_pools.c:691: too few arguments to function `apr_make_sub_pool'
Argh, patch on the way. I had one more thing to commit, and I forgot to
do it before I left the office. Sorry :-(
Ryan
___
Ryan Bloom [EMAIL PROTECTED]
406 29th St.
San Francisco, CA 94131
---
Re: cvs commit: apr/lib apr_pools.c
Before anybody yells that this does a lot, I'm backing out 99% of this. Most of this wasn't readdy yet, I forgot to put configure.in on my commit line. :-( Ryan On 5 Jan 2001 [EMAIL PROTECTED] wrote: > rbb 01/01/04 16:10:11 > > Modified:.configure.in >file_io/unix fileacc.c >include apr_file_io.h >lib apr_pools.c > Log: > We need to initialize have_corkable_tcp to 0, otherwise on platforms > that can't cork, we get a syntax error when we check #if HAVE_CORKABLE_TCP, > because HAVE_CORKABLE_TCP is empty ___ Ryan Bloom [EMAIL PROTECTED] 406 29th St. San Francisco, CA 94131 ---
RE: cvs commit: apr/lib apr_pools.c
> -Original Message- > From: [EMAIL PROTECTED] [mailto:[EMAIL PROTECTED] > Sent: Monday, January 01, 2001 8:33 PM > To: [EMAIL PROTECTED] > Subject: cvs commit: apr/lib apr_pools.c > > > rbb 01/01/01 17:33:05 > > Modified:.CHANGES >lib apr_pools.c > Log: > Remove the ability to allocate out of a NULL pool. This was always a bad > idea, because it opened up memory leaks. It is very likely that this will > expose some seg faults This is indeed true, now that Windows httpd-2.0 is building again it has exposed a couple of problems. In ap_is_rdirectory, apr_lstat is passed a NULL pool pointer which eventually causes apr_palloc to segfault. I assume the correct fix is to pass in a pool parameter to ap_is_rdirectory. Similar situation seems to exist with apr_stat being passed a NULL pool by ap_is_directory. I'll go ahead and fix these. Allan
RE: cvs commit: apr/lib apr_pools.c
> > Remove the ability to allocate out of a NULL pool. This was always a bad > > idea, because it opened up memory leaks. It is very likely that this will > > expose some seg faults > > This is indeed true, now that Windows httpd-2.0 is building again it has > exposed a couple of problems. In ap_is_rdirectory, apr_lstat is passed > a NULL pool pointer which eventually causes apr_palloc to segfault. > > I assume the correct fix is to pass in a pool parameter to ap_is_rdirectory. > Similar situation seems to exist with apr_stat being passed a NULL pool > by ap_is_directory. I'll go ahead and fix these. I disagree that this is the correct solution. APR relies too heavily on pools right now, and I think this is causing some of our memory leak. For example, why does apr_get_oslevel take a pool as an argument? It doesn't use it ever. That pool should go away. Once that is done, the pool can be removed from apr_stat and apr_lstat. As far as I can see, the pools aren't used there either. Ryan ___ Ryan Bloom [EMAIL PROTECTED] 406 29th St. San Francisco, CA 94131 ---
RE: cvs commit: apr/lib apr_pools.c
> > I assume the correct fix is to pass in a pool parameter to ap_is_rdirectory. > > Similar situation seems to exist with apr_stat being passed a NULL pool > > by ap_is_directory. I'll go ahead and fix these. > > I disagree that this is the correct solution. APR relies too heavily on > pools right now, and I think this is causing some of our memory leak. For > example, why does apr_get_oslevel take a pool as an argument? It doesn't I can't answer for apr_get_oslevel, maybe the pool should go away in that case. However both apr_stat and apr_lstat need the pool on Windows because of the utf8_to_unicode_path call which does use it. > use it ever. That pool should go away. Once that is done, the pool can > be removed from apr_stat and apr_lstat. As far as I can see, the pools > aren't used there either. > Allan
RE: cvs commit: apr/lib apr_pools.c
> > I disagree that this is the correct solution. APR relies too heavily on > > pools right now, and I think this is causing some of our memory leak. For > > example, why does apr_get_oslevel take a pool as an argument? It doesn't > > I can't answer for apr_get_oslevel, maybe the pool should go away in that > case. > However both apr_stat and apr_lstat need the pool on Windows because of the > utf8_to_unicode_path call which does use it. Yep, I missed those calls. Ryan ___ Ryan Bloom [EMAIL PROTECTED] 406 29th St. San Francisco, CA 94131 ---
Re: cvs commit: apr/lib apr_pools.c
On Sat, Jan 06, 2001 at 03:18:48PM -, [EMAIL PROTECTED] wrote: > rbb 01/01/06 07:18:48 > > Modified:.CHANGES >lib apr_pools.c > Log: > Keep apr_terminate from seg faulting on terminate. This is > happening on systems that do not NULL out locks when they are > destroyed. To keep this from happening, we set the locks to > NULL after destroying them in apr_terminate, and we have to > check for NULL in free_blocks. >... > @@ -702,6 +706,8 @@ >#if APR_HAS_THREADS >apr_destroy_lock(alloc_mutex); >apr_destroy_lock(spawn_mutex); > +alloc_mutex = NULL; > +spawn_mutex = NULL; >#endif >apr_destroy_pool(globalp); >} To be clear, it doesn't matter what platform it is: nobody can "NULL out locks" in the above case. apr_destroy_lock() can't reach out and set alloc_mutex to NULL unless the call pattern is &alloc_mutex. IOW, this isn't platform dependent. It may simply be that other platforms are being a bit more tolerant; that the lock is still sitting around in a useful format in free'd memory. Cheers, -g -- Greg Stein, http://www.lyra.org/
Re: cvs commit: apr/lib apr_pools.c
> > @@ -702,6 +706,8 @@ > >#if APR_HAS_THREADS > >apr_destroy_lock(alloc_mutex); > >apr_destroy_lock(spawn_mutex); > > +alloc_mutex = NULL; > > +spawn_mutex = NULL; > >#endif > >apr_destroy_pool(globalp); > >} > > To be clear, it doesn't matter what platform it is: nobody can "NULL out > locks" in the above case. apr_destroy_lock() can't reach out and set > alloc_mutex to NULL unless the call pattern is &alloc_mutex. > > IOW, this isn't platform dependent. It may simply be that other platforms > are being a bit more tolerant; that the lock is still sitting around in a > useful format in free'd memory. Good point. Ryan ___ Ryan Bloom [EMAIL PROTECTED] 406 29th St. San Francisco, CA 94131 ---
Re: cvs commit: apr/lib apr_pools.c
[EMAIL PROTECTED] wrote: > > Add a nifty memory/pool debugging option: allocation everything on a page, > then turn the page inaccessible at "free" time. Causes segfaults on access. > way cool! Thanks. Greg
