On Sun, Feb 11, 2001 at 06:13:21PM -0500, Gianni Johansson wrote:
> Hi Scott:
> 
> I was looking at ThreadPool.reclaim today and I found something that just 
> doesn't look right.
> 
>  /**
>      * Called by an EThread to indicate that it has finished processing
>      * a job and may be reclaimed by another job.
>      *
>      * @param e The EThread to reclaim.
>      */
>     public boolean reclaim(EThread e) {
>       boolean rv=false;
>       if (maxThreads == 0 || threads.size()<maxPoolThreads) {
>             // LOOK HERE ---------------^
>           threads.push(e);
>           rv=true;
>       } 
> 
>       synchronized(threads) {
>           threads.notifyAll();
>       }
>       return rv;
>     }
> 
> Shouldn't the check on threads.size() be against maxThreads?
> 
> I couldn't figure out what the intent of maxPoolThreads is.
> The value is hard coded to 5 in the ThreadPool constructor call in Core.Core.

The maxPoolThreads value specifies how many threads to keep around in the
pool idly.  The test says "If you already have more than maxPoolThreads in
the pool, discard this one, since there are already that many waiting to
service requests"


> 
> If I am following the current implementation of ThreadPool correctly,  it 
> looks like you will start out with maxThreads Ethread and discard them
> as they are used up, until only maxPoolThreads (5 in the current code) remain.
maxThreads specifies the maximum running at any time.

> 
> This would explain the SendFailedExceptions when making lots of concurrent 
> connections, that I reported earlier on this list.
No, that wouldn't do that.

> Also, it looks like the entire method should be synchronized on threads, 
> otherwise there's a race condition.
No, because the worst thing that could happen in a race is one fewer
thread in the pool.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 232 bytes
Desc: not available
URL: 
<https://emu.freenetproject.org/pipermail/devl/attachments/20010211/a58e4022/attachment.pgp>

Reply via email to