You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@apr.apache.org by dean gaudet <de...@arctic.org> on 2001/07/08 20:34:25 UTC
multithreaded pools?
so while hacking up that patch to eliminate the block_freelist i thought i
could also get rid of the alloc_mutex entirely. but it appears that
currently the pool code sort of supports multithreaded access to the same
pool.
i say "sort of" because cleanups don't support multithreaded access -- no
mutex there. and cleanups of pools across threads sounds really ugly
(because we deliberately don't have async notification of threads).
but if you study the parent/child pool linking code (search for ->parent)
you'll see that whenever the parent pool is touched, it's done inside
alloc_mutex.
ages ago it was discussed that there would be a "root" pool per-thread --
and that a pool would be accessed by a single thread. this would mean
that alloc_mutex wouldn't need to be aquired to touch the parent pool.
ok studying the mpm threaded.c code i see that we give each thread a
sub_pool of pchild. but i think the following patch would be safe,
because each thread won't exit until it has done its own cleanup.
i haven't tested the patch though... what do people think?
(the real testing would include removing the alloc_mutex from apr_pools.c
code which accesses p->parent.)
-dean
Index: threaded.c
===================================================================
RCS file: /home/cvs/httpd-2.0/server/mpm/threaded/threaded.c,v
retrieving revision 1.44
diff -u -r1.44 threaded.c
--- threaded.c 2001/07/03 13:58:10 1.44
+++ threaded.c 2001/07/08 18:32:50
@@ -697,7 +697,9 @@
my_info->pid = my_child_num;
my_info->tid = i;
my_info->sd = 0;
- apr_pool_create(&my_info->tpool, pchild);
+
+ /* each thread needs its own private pool tree */
+ apr_pool_create(&my_info->tpool, NULL);
/* We are creating threads right now */
(void) ap_update_child_status(my_child_num, i, SERVER_STARTING,
Re: multithreaded pools?
Posted by Luke Kenneth Casson Leighton <lk...@samba-tng.org>.
On Sun, Jul 08, 2001 at 10:36:10PM -0700, Roy T. Fielding wrote:
> OTOH, it seems the new sms code just needs a whack upside the head to
> stop it from asking for extra memory from its parent.
*cackle*.
just got in, saw a whole boat-load of messages about sms [grief,
who'd have thought it, neh? :) will get to them in a bit].
i saw the profiling: a suggestion - why not make a less-trivial
sms that uses a hash-table? i mean, the trivial sms proves
the point, and people can now go 'oh yeah, it works!' [and,
'what was all the darn fuss about???']. 'now, how do we make
it better, little-step-by-little-step'?
sander, did you do sma as an sms yet (sma: smart memory allocator),
to do a comparison?
luke
Re: multithreaded pools?
Posted by rb...@covalent.net.
On Tue, 10 Jul 2001, dean gaudet wrote:
> On Sun, 8 Jul 2001, Roy T. Fielding wrote:
>
> [clean_child_exit]
> >
> > It is only called when the child exits and not per-thread. I think the
> > threads are already dead by that point, or locked-up due to some fatal
> > error that is the reason why clean_child_exit is being called.
>
> when you say "the threads are dead" you're referring to the httpd threads.
> there could be zillions of other threads started up by third party
> modules, still running.
>
> but also, if you look at the call sites within threaded.c you'll find that
> even the httpd threads are (potentially) still running.
>
> huh, at least one of the call sites -- where apr_thread_create fails looks
> like a genuine bug. exit the PROCESS if you can't create a thread, for
> any reason whatsoever? this is in start_threads.
>
> also, clean_child_exit is called when SIGTERM is received... and makes no
> attempt to stop the httpd threads.
I know it looks like that when you first look through the code, but
receiving a SIGTERM should not execute clean_child_exit. We have a single
thread in sigwait, which means that all signals should be blocked in all
other threades. The one sigwait thread then gets the signal, and waits
for all the threads to die before it calls clean_child_exit.
Now, having reviewed the code, this looks broken, because we are setting a
signal handler, and I don't see us blocking the signals anywhere, but that
was the design anyway. :-(
>
> so basically clean_child_exit() really should just call exit() and avoid
> the possibility of damaging data by calling cleanups owned by other
> threads (and with no locking to protect them).
Ryan
_____________________________________________________________________________
Ryan Bloom rbb@apache.org
Covalent Technologies rbb@covalent.net
-----------------------------------------------------------------------------
Re: multithreaded pools?
Posted by rb...@covalent.net.
On Tue, 10 Jul 2001, dean gaudet wrote:
> On Sun, 8 Jul 2001, Roy T. Fielding wrote:
>
> [clean_child_exit]
> >
> > It is only called when the child exits and not per-thread. I think the
> > threads are already dead by that point, or locked-up due to some fatal
> > error that is the reason why clean_child_exit is being called.
>
> when you say "the threads are dead" you're referring to the httpd threads.
> there could be zillions of other threads started up by third party
> modules, still running.
>
> but also, if you look at the call sites within threaded.c you'll find that
> even the httpd threads are (potentially) still running.
>
> huh, at least one of the call sites -- where apr_thread_create fails looks
> like a genuine bug. exit the PROCESS if you can't create a thread, for
> any reason whatsoever? this is in start_threads.
>
> also, clean_child_exit is called when SIGTERM is received... and makes no
> attempt to stop the httpd threads.
I know it looks like that when you first look through the code, but
receiving a SIGTERM should not execute clean_child_exit. We have a single
thread in sigwait, which means that all signals should be blocked in all
other threades. The one sigwait thread then gets the signal, and waits
for all the threads to die before it calls clean_child_exit.
Now, having reviewed the code, this looks broken, because we are setting a
signal handler, and I don't see us blocking the signals anywhere, but that
was the design anyway. :-(
>
> so basically clean_child_exit() really should just call exit() and avoid
> the possibility of damaging data by calling cleanups owned by other
> threads (and with no locking to protect them).
Ryan
_____________________________________________________________________________
Ryan Bloom rbb@apache.org
Covalent Technologies rbb@covalent.net
-----------------------------------------------------------------------------
Re: multithreaded pools?
Posted by dean gaudet <de...@arctic.org>.
On Sun, 8 Jul 2001, Roy T. Fielding wrote:
[clean_child_exit]
>
> It is only called when the child exits and not per-thread. I think the
> threads are already dead by that point, or locked-up due to some fatal
> error that is the reason why clean_child_exit is being called.
when you say "the threads are dead" you're referring to the httpd threads.
there could be zillions of other threads started up by third party
modules, still running.
but also, if you look at the call sites within threaded.c you'll find that
even the httpd threads are (potentially) still running.
huh, at least one of the call sites -- where apr_thread_create fails looks
like a genuine bug. exit the PROCESS if you can't create a thread, for
any reason whatsoever? this is in start_threads.
also, clean_child_exit is called when SIGTERM is received... and makes no
attempt to stop the httpd threads.
so basically clean_child_exit() really should just call exit() and avoid
the possibility of damaging data by calling cleanups owned by other
threads (and with no locking to protect them).
...
anyhow, you've got issues with root pools coming out our ears. i can
understand that. a solution which came up ages ago when we first talked
about this was to register a "thread cleanup" in the parent. the thread
cleanup is a link to the thread's root pool, and because it doesn't go
through the usual parent/child relationship we can handle it specially
(such as including locking while accessing it, this avoids hitting the
fast path that we want there for all other sub pool create/destroys).
now we just need to figure out how to get the thread to rendezvous with
the parent and we can build a cleanup system that works :)
while i am wholly against async notification, i'm not at all against a
unified method of synchronous notification -- a flag somewhere in the
per-thread data indicating "please stop at your earliest convenience".
but i'm still not sure this is useful for clean_child_exit --
clean_child_exit is a lot like the debate about abort() within apr_palloc.
something is royally screwed when this happens, and there's really nothing
graceful you can do... i believe the safest thing is to call exit() as
fast as possible without stopping to try to do anything fancy.
-dean
Re: multithreaded pools?
Posted by dean gaudet <de...@arctic.org>.
On Sun, 8 Jul 2001, Roy T. Fielding wrote:
[clean_child_exit]
>
> It is only called when the child exits and not per-thread. I think the
> threads are already dead by that point, or locked-up due to some fatal
> error that is the reason why clean_child_exit is being called.
when you say "the threads are dead" you're referring to the httpd threads.
there could be zillions of other threads started up by third party
modules, still running.
but also, if you look at the call sites within threaded.c you'll find that
even the httpd threads are (potentially) still running.
huh, at least one of the call sites -- where apr_thread_create fails looks
like a genuine bug. exit the PROCESS if you can't create a thread, for
any reason whatsoever? this is in start_threads.
also, clean_child_exit is called when SIGTERM is received... and makes no
attempt to stop the httpd threads.
so basically clean_child_exit() really should just call exit() and avoid
the possibility of damaging data by calling cleanups owned by other
threads (and with no locking to protect them).
...
anyhow, you've got issues with root pools coming out our ears. i can
understand that. a solution which came up ages ago when we first talked
about this was to register a "thread cleanup" in the parent. the thread
cleanup is a link to the thread's root pool, and because it doesn't go
through the usual parent/child relationship we can handle it specially
(such as including locking while accessing it, this avoids hitting the
fast path that we want there for all other sub pool create/destroys).
now we just need to figure out how to get the thread to rendezvous with
the parent and we can build a cleanup system that works :)
while i am wholly against async notification, i'm not at all against a
unified method of synchronous notification -- a flag somewhere in the
per-thread data indicating "please stop at your earliest convenience".
but i'm still not sure this is useful for clean_child_exit --
clean_child_exit is a lot like the debate about abort() within apr_palloc.
something is royally screwed when this happens, and there's really nothing
graceful you can do... i believe the safest thing is to call exit() as
fast as possible without stopping to try to do anything fancy.
-dean
Re: multithreaded pools?
Posted by "Roy T. Fielding" <fi...@ebuilt.com>.
On Sun, Jul 08, 2001 at 11:23:56PM -0700, dean gaudet wrote:
> On Sun, 8 Jul 2001, Roy T. Fielding wrote:
>
> > The last time I looked at the pool code it was bogus because clean_child_exit
>
> how can clean_child_exit ever hope to work in a multithreaded server
> without async notification?
It is only called when the child exits and not per-thread. I think the
threads are already dead by that point, or locked-up due to some fatal
error that is the reason why clean_child_exit is being called.
> in particular, what happens when modules start creating their own thread
> pools within the server to do their own specific stuff?
I presumed they would be children of the per-thread root pool.
> i don't see how my change would make it any worse. at least for the httpd
> threads you know how to tell them all to finish up and die, and on the way
> out they happen to destroy their pools. so that handles all those new
> pool roots. (i'm not sure if clean_child_exit does this at all
> currently.)
Oh, only worse in the sense that it adds more pools that are disconnected,
yet dependent upon, the global pool created during apr_initialize. I just
think we should fix that bug first -- either connect them all or remove the
dependencies and make sure the cleanups still work.
> if clean_child_exit goes happily waltzing all over data in pools that are
> in use by other threads without first killing the other threads then we're
> just asking for segfaults, or even worse: data corruption (such as
> calling a cleanup on something which was unregistered). there's race
> conditions left and right. you'd kind of have to lock everything
> always... and then you'd have to deal with deadlocks on the way out of
> clean_child_exit (not to mention a lack of performance).
I think it is only run after the threads are dead, but I don't know the
threaded code well enough to know if the threads are actually stopped
in a clean way or if it still assumes that the process can clean up
everything that the threads needed to clean.
The reason I said it was already broken is because the disconnected
pools meant that some of the subpools that were being automatically
cleaned-up on exit (because they descended from pglobal in 1.3) were
not being cleaned-up in 2.0. Their cleanups simply weren't being called.
At least not as far as I could see (which wasn't all that far because the
threaded MPM code gives me a headache).
....Roy
Re: multithreaded pools?
Posted by Luke Kenneth Casson Leighton <lk...@samba-tng.org>.
On Sun, Jul 08, 2001 at 10:36:10PM -0700, Roy T. Fielding wrote:
> OTOH, it seems the new sms code just needs a whack upside the head to
> stop it from asking for extra memory from its parent.
*cackle*.
just got in, saw a whole boat-load of messages about sms [grief,
who'd have thought it, neh? :) will get to them in a bit].
i saw the profiling: a suggestion - why not make a less-trivial
sms that uses a hash-table? i mean, the trivial sms proves
the point, and people can now go 'oh yeah, it works!' [and,
'what was all the darn fuss about???']. 'now, how do we make
it better, little-step-by-little-step'?
sander, did you do sma as an sms yet (sma: smart memory allocator),
to do a comparison?
luke
Re: multithreaded pools?
Posted by "Roy T. Fielding" <fi...@ebuilt.com>.
On Sun, Jul 08, 2001 at 11:23:56PM -0700, dean gaudet wrote:
> On Sun, 8 Jul 2001, Roy T. Fielding wrote:
>
> > The last time I looked at the pool code it was bogus because clean_child_exit
>
> how can clean_child_exit ever hope to work in a multithreaded server
> without async notification?
It is only called when the child exits and not per-thread. I think the
threads are already dead by that point, or locked-up due to some fatal
error that is the reason why clean_child_exit is being called.
> in particular, what happens when modules start creating their own thread
> pools within the server to do their own specific stuff?
I presumed they would be children of the per-thread root pool.
> i don't see how my change would make it any worse. at least for the httpd
> threads you know how to tell them all to finish up and die, and on the way
> out they happen to destroy their pools. so that handles all those new
> pool roots. (i'm not sure if clean_child_exit does this at all
> currently.)
Oh, only worse in the sense that it adds more pools that are disconnected,
yet dependent upon, the global pool created during apr_initialize. I just
think we should fix that bug first -- either connect them all or remove the
dependencies and make sure the cleanups still work.
> if clean_child_exit goes happily waltzing all over data in pools that are
> in use by other threads without first killing the other threads then we're
> just asking for segfaults, or even worse: data corruption (such as
> calling a cleanup on something which was unregistered). there's race
> conditions left and right. you'd kind of have to lock everything
> always... and then you'd have to deal with deadlocks on the way out of
> clean_child_exit (not to mention a lack of performance).
I think it is only run after the threads are dead, but I don't know the
threaded code well enough to know if the threads are actually stopped
in a clean way or if it still assumes that the process can clean up
everything that the threads needed to clean.
The reason I said it was already broken is because the disconnected
pools meant that some of the subpools that were being automatically
cleaned-up on exit (because they descended from pglobal in 1.3) were
not being cleaned-up in 2.0. Their cleanups simply weren't being called.
At least not as far as I could see (which wasn't all that far because the
threaded MPM code gives me a headache).
....Roy
Re: multithreaded pools?
Posted by dean gaudet <de...@arctic.org>.
On Sun, 8 Jul 2001, Roy T. Fielding wrote:
> The last time I looked at the pool code it was bogus because clean_child_exit
how can clean_child_exit ever hope to work in a multithreaded server
without async notification?
in particular, what happens when modules start creating their own thread
pools within the server to do their own specific stuff?
i don't see how my change would make it any worse. at least for the httpd
threads you know how to tell them all to finish up and die, and on the way
out they happen to destroy their pools. so that handles all those new
pool roots. (i'm not sure if clean_child_exit does this at all
currently.)
if clean_child_exit goes happily waltzing all over data in pools that are
in use by other threads without first killing the other threads then we're
just asking for segfaults, or even worse: data corruption (such as
calling a cleanup on something which was unregistered). there's race
conditions left and right. you'd kind of have to lock everything
always... and then you'd have to deal with deadlocks on the way out of
clean_child_exit (not to mention a lack of performance).
-dean
Re: multithreaded pools?
Posted by dean gaudet <de...@arctic.org>.
On Sun, 8 Jul 2001, Roy T. Fielding wrote:
> The last time I looked at the pool code it was bogus because clean_child_exit
how can clean_child_exit ever hope to work in a multithreaded server
without async notification?
in particular, what happens when modules start creating their own thread
pools within the server to do their own specific stuff?
i don't see how my change would make it any worse. at least for the httpd
threads you know how to tell them all to finish up and die, and on the way
out they happen to destroy their pools. so that handles all those new
pool roots. (i'm not sure if clean_child_exit does this at all
currently.)
if clean_child_exit goes happily waltzing all over data in pools that are
in use by other threads without first killing the other threads then we're
just asking for segfaults, or even worse: data corruption (such as
calling a cleanup on something which was unregistered). there's race
conditions left and right. you'd kind of have to lock everything
always... and then you'd have to deal with deadlocks on the way out of
clean_child_exit (not to mention a lack of performance).
-dean
Re: multithreaded pools?
Posted by "Roy T. Fielding" <fi...@ebuilt.com>.
> ok studying the mpm threaded.c code i see that we give each thread a
> sub_pool of pchild. but i think the following patch would be safe,
> because each thread won't exit until it has done its own cleanup.
The last time I looked at the pool code it was bogus because clean_child_exit
assumed every pool was descended from the process root and several new
parentless pools had been added to the code -- I never got around to fixing
that, but this change would make it worse. I think it would be better to
remove the global pool free list so that the application could init multiple
root pools (each with their own root free list, or with no free list if we
can show that to be more efficient).
OTOH, it seems the new sms code just needs a whack upside the head to
stop it from asking for extra memory from its parent.
....Roy
Re: multithreaded pools?
Posted by "Roy T. Fielding" <fi...@ebuilt.com>.
> ok studying the mpm threaded.c code i see that we give each thread a
> sub_pool of pchild. but i think the following patch would be safe,
> because each thread won't exit until it has done its own cleanup.
The last time I looked at the pool code it was bogus because clean_child_exit
assumed every pool was descended from the process root and several new
parentless pools had been added to the code -- I never got around to fixing
that, but this change would make it worse. I think it would be better to
remove the global pool free list so that the application could init multiple
root pools (each with their own root free list, or with no free list if we
can show that to be more efficient).
OTOH, it seems the new sms code just needs a whack upside the head to
stop it from asking for extra memory from its parent.
....Roy