apr-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Davi Arnaut <d...@haxent.com.br>
Subject Re: [Fwd: Re: Ack: CLA for Henry Jen [was Re: thread pool status]]
Date Tue, 23 May 2006 00:22:52 GMT
On Mon, 22 May 2006 15:07:56 -0700
Henry Jen <henryjen@ztune.net> wrote:

> William A. Rowe, Jr. wrote:
> > +notinavail:Henry Jen:Henry Jen:henryjen@ztune.net: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 ?

+
+    *me = apr_pcalloc(pool, sizeof(**me));
+    if (!*me) {
+        return APR_ENOMEM;
+    }

Be concise, either check all apr_pcalloc failures or none.

+
+    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 :)

+        ++me->thd_cnt;
+        apr_thread_create(&thd, NULL, thread_pool_func, me, me->pool);
+    }

apr_thread_create may fail..

--
Davi Arnaut


Mime
View raw message