Here is a proposed patch, tell me what you think. (note this also goes back to wait(1000) once every loop of the creation thread, I did this to ensure that one call to awaken() will _always_ put the thread in creation mode.
--Brandon On Sat, 09/27/03 at 10:47:30 -0500, Brandon Low wrote: > Ok, so I'm reading QThreadFactory ONE MORE TIME... There may be a > problem, we might _want to_ overestimate available during create thread, > return thread, but then underestimate it in destroy thread and get > thread, because of the following: what happens if by (relatively > impossible, but it is _theoretically_ possible, and as freenet scales it > will happen) some chance, the QThreadFactory's creator thread is able to > keep calling createThread repeatedly but NONE of those threads that are > created ever manage to get the countLock? Then all of a sudden we have > tons of extra threads around... createThread should inc. available as > soon as it fires IMHO. > > Thoughts? > > Thanks, > > Brandon > _______________________________________________ > Devl mailing list > [EMAIL PROTECTED] > http://dodo.freenetproject.org/cgi-bin/mailman/listinfo/devl
Index: src/freenet/thread/QThreadFactory.java
===================================================================
RCS file: /cvsroot/freenet/freenet/src/freenet/thread/QThreadFactory.java,v
retrieving revision 1.35
diff -u -r1.35 QThreadFactory.java
--- src/freenet/thread/QThreadFactory.java 27 Sep 2003 07:37:16 -0000 1.35
+++ src/freenet/thread/QThreadFactory.java 27 Sep 2003 16:00:50 -0000
@@ -29,7 +29,8 @@
private final CountLock countLock = new CountLock();
private int active = 0;
- private int available = 0; // never more than number of threads on stack.
+ private int available = 0; //Must always include threads who's creations
+ //or deletions are currently happening!
private final NumLock numLock = new NumLock();
private long threadNumber = 0;
@@ -80,19 +81,6 @@
Math.min(required,
Math.max((int) (active * IDEAL_AVAILABLE_RATIO),
2 * MINIMUM_AVAILABLE_ABS));
- if ( available > required &&
- available > active *
MINIMUM_AVAILABLE_RATIO ) {
- try {
- countLock.wait(500);
- } catch ( InterruptedException
ie ) {
- // active might have
changed.
- // required must not
increase.
- required =
-
Math.min(required,
-
Math.max((int) (active * IDEAL_AVAILABLE_RATIO),
-
2 * MINIMUM_AVAILABLE_ABS));
- }
- }
logAvailable = available;
logActive = active;
}
@@ -127,19 +115,6 @@
Math.max((int) (active *
IDEAL_AVAILABLE_RATIO),
Math.max(allowed,
2 * MINIMUM_AVAILABLE_ABS));
- if ( available < allowed &&
- available < active *
MAXIMUM_AVAILABLE_RATIO ) {
- try {
- countLock.wait(500);
- } catch ( InterruptedException
ie ) {
- // active might have
changed.
- // allowed must not
decrease.
- allowed =
- Math.max((int)
(active * IDEAL_AVAILABLE_RATIO),
-
Math.max(allowed,
-
2 * MINIMUM_AVAILABLE_ABS));
- }
- }
logAvailable = available;
logActive = active;
}
@@ -168,7 +143,7 @@
}
synchronized(countLock) {
try {
- countLock.wait(500);
+ countLock.wait(1000);
} catch ( InterruptedException ie ) {}
}
}
@@ -210,7 +185,10 @@
QThread thread = null;
synchronized(countLock) {
active++;
- available--; // never an overestimate
+ available--; //Aleays an underestimate
+ //to ensure that once a get
+ //starts it's thread is
+ //replaced.
}
synchronized(headLock) {
// hopefully this will not happen often.
@@ -228,7 +206,9 @@
newThreadNumber = ++threadNumber;
}
synchronized(countLock) {
- available++; // correct the underestimate.
+ available++; //correct the underestimate
+ //_before_ actually creating
+ //the thread!
}
thread = new QThread(newThreadNumber);
}
@@ -249,14 +229,17 @@
synchronized(numLock) {
newThreadNumber = ++threadNumber;
}
+ synchronized(countLock) {
+ available++; //Always overestimate here
+ //We don't want to spin
+ //Starting more and more
+ //creations w/o counting
+ }
newThread = new QThread(newThreadNumber);
synchronized(headLock) {
newThread.next = headerThread;
headerThread = newThread;
}
- synchronized(countLock) {
- available++; // never an overestimate.
- }
}
/**
@@ -266,14 +249,17 @@
*/
private final void destroyThread() {
QThread dyingThread;
- synchronized(countLock) {
- if (available < 1) return;
- available--; // never an overestimate.
- }
synchronized(headLock) {
dyingThread = headerThread;
headerThread = headerThread.next;
}
+ synchronized(countLock) {
+ if (available < 1) return;
+ available--; //Always underestimate on
+ //deletions, we don't want
+ //to spin starting more
+ //deletions
+ }
dyingThread.die();
}
@@ -281,13 +267,13 @@
* Returns a thread to the stack when it is finished executing.
*/
private final void returnThread(QThread returnThread) {
+ synchronized(countLock) {
+ active--;
+ available++; //Always an overestimate
+ }
synchronized(headLock) {
returnThread.next = headerThread;
headerThread = returnThread;
- }
- synchronized(countLock) {
- active--;
- available++; // never an overestimate
}
}
pgp00000.pgp
Description: PGP signature
_______________________________________________ Devl mailing list [EMAIL PROTECTED] http://dodo.freenetproject.org/cgi-bin/mailman/listinfo/devl
