Davi Arnaut wrote:
On Mon, 22 May 2006 15:07:56 -0700
Henry Jen <[EMAIL PROTECTED]> wrote:
William A. Rowe, Jr. wrote:
+notinavail:Henry Jen:Henry Jen:[EMAIL PROTECTED]:Signed CLA
Is received - for the benefit of anyone with cycles to commit his thread
pool design or any other code contributions.
Attached is the latest patch for the thread pool implementation, your
help on review/commit it is greatly appreciated.
Cheers,
Henry
Glimpse:
+static apr_status_t thread_pool_cleanup(void *me)
+{
+ apr_thread_pool_t *_self = me;
+
+ _self->terminated = 1;
+ apr_thread_pool_idle_max_set(_self, 0);
+ while (_self->thd_cnt) {
+ apr_sleep(20 * 1000); /* spin lock with 20 ms */
+ }
What happens to the busy threads ?
Those will stop after done current task at hand with the terminated
flag. Therefore a spin lock. :-)
+
+ *me = apr_pcalloc(pool, sizeof(**me));
+ if (!*me) {
+ return APR_ENOMEM;
+ }
Be concise, either check all apr_pcalloc failures or none.
I intent to check, I spot one place I did not in thread_pool_func,
anywhere else you noticed?
It was brought up earlier that apache has a convention not to check, but
some application needs to be able to stop gracefully even with error
condition happens. So, is checking acceptable to apr?
+
+ while (init_threads) {
+ ++(*me)->thd_cnt;
+ rv = apr_thread_create(&t, NULL, thread_pool_func, *me, (*me)->pool);
+ if (APR_SUCCESS == rv) {
+ --init_threads;
+ }
+ }
What if apr_thread_create fails ? thd_cnt will hold a wrong value. Also, a small
apr_sleep() here wouldn't hurt.
+ if (0 > next) {
Excuse me, but this is just too much :)
Okay, okay. I will change it to 'if (next < 0)'. :-)
+ ++me->thd_cnt;
+ apr_thread_create(&thd, NULL, thread_pool_func, me, me->pool);
+ }
apr_thread_create may fail..
Thanks, I will fix it.
Cheers,
Henry
