I have been researching the implementation of NDC::remove and
NDC::lazyRemove. I think that I may have found a bug in the implementation
of NDC::lazyRemove, however, I may be missing something.
I wanted to know how often the main clean up portion of NDC::lazyRemove was
to be executed. From the code, I see that NDC::lazyRemove is only called by
NDC::remove. NDC::remove calls NDC::lazyRemove every time it is called. I
believe that the implementation of lazyRemove is intended to have the main
clean up portion run only every sixth time it is called, however, I believe
that the "if" statement controlling this is wrong. Here is the source code
from NDC::lazyRemove
private
static
void lazyRemove() {
// The synchronization on ht is necessary to prevent JDK 1.2.x from
// throwing ConcurrentModificationExceptions at us. This sucks BIG-TIME.
// One solution is to write our own hashtable implementation.
Vector v;
synchronized(ht) {
// Avoid calling clean-up too often.
if(++pushCounter >= REAP_THRESHOLD) {
return; // We release the lock ASAP.
} else {
pushCounter = 0; // OK let's do some work.
}
int misses = 0;
v = new Vector();
Enumeration enum = ht.keys();
// We give up after 4 straigt missses. That is 4 consecutive
// inspected threads in 'ht' that turn out to be alive.
// The higher the proportion on dead threads in ht, the higher the
// chances of removal.
while(enum.hasMoreElements() && (misses <= 4)) {
Thread t = (Thread) enum.nextElement();
if(t.isAlive()) {
misses++;
} else {
misses = 0;
v.addElement(t);
}
}
} // synchronized
int size = v.size();
for(int i = 0; i < size; i++) {
Thread t = (Thread) v.elementAt(i);
LogLog.debug("Lazy NDC removal for thread [" + t.getName() + "] ("+
ht.size() + ").");
ht.remove(t);
}
}
The if statement is executed only if pushCounter is >= REAP_THRESHOLD
<<<<< if(++pushCounter >= REAP_THRESHOLD) { >>>>>>
however, pushCounter is initialized to 0 and only ever incremented here.
REAP_THRESHOLD is initialized to 5, and never modified (it is static final).
The first time NDC::lazyRemove is called, pushCounter will be 0. Since 0 is
less than 5, the else statement is executed. The else statement then sets
pushCounter back to 0, so that every time lazyRemove is called, pushCounter
is always 0, meaning that the clean up code is ALWAYS executed, not every
sixth time.
Is the if statement supposed to be less than or equal to instead of greater
than or equal to, like this <<<<<<<<< if(++pushCounter <= REAP_THRESHOLD) {
>>>>>?
Thoughts?????
---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
