On Sunday 11 February 2001 20:24, Scott wrote:

OK.  There's still a bug, but it didn't quite describe it right.  Look at the 
implementation for runningThreads:

 private int runningThreads() {
        return maxThreads-threads.size();
    }

You are depending on the size of threads to keep track of how many
threads are running.  But that would only make sense if you put the used
EThreads back into threads in reclaim() which you don't do.

threads shrinks as the EThreads initially created fillThreadStack get used up 
without getting push()'d back in. 

runningThreads() reports an erroneously high value, until you reach a point 
where every single call to run() fails, even though less than maxThreads jobs
are running.

That's what's causing the SendFailedExceptions.

-- gj

    /**
     * Queue a job for running.
     *
     * @param r A job to queue.
     *
     * @return True if the job was successfully queued, or false
     * if this ThreadPool is not running or if the maximum number of
     * allowable jobs for this ThreadPool has been reached.
     */
    public boolean run(Runnable r) {
        if (run) {
            //      
System.err.println("["+runningThreads()+','+jobs.size()+']');
            if (runningThreads() + jobs.size() < maxJobs) {
                jobs.enqueue(r);
                return true;
            }
        }
        return false;
    }



> > 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.

----------------------------------------
Content-Type: application/pgp-signature; charset="us-ascii"; 
name="Attachment: 1"
Content-Transfer-Encoding: 7bit
Content-Description: 
----------------------------------------

-- 
Web page inside Freenet:
freenet:MSK at SSK@enI8YFo3gj8UVh-Au0HpKMftf6QQAgE/homepage// 

_______________________________________________
Devl mailing list
Devl at freenetproject.org
http://www.uprizer.com/mailman/listinfo/devl

Reply via email to