Blocked in StdServerSession.start: PooledExecutor has no thr

[thoennes]

Hi,

we have seen a related post recently in this forum, where genman analyzed the situation and
added a fix in the HEAD of Branch_3_2:

revision 1.14.2.3
date: 2004/01/13 19:21:25; author: genman; state: Exp; lines: +21 -1
Throwable / Thread.interrupted must be caught/checked otherwise we lose JMS
ServerSession threads through PooledExecutor
(Looking at the sources, this sort of safety checking is done elsewhere.)
This is a fix for a "forum bug"
http://jboss.org/index.html?module=bb&op=viewtopic&p=3818218#3818218

The forum entry disappeared and therefore we created a new topic.

Our MDB is configured as a singleton (MaximumSize=1), so that the PooledExecutor
gets configured as follows:

executor = new PooledExecutor(poolSize); // poolSize = 1
executor.setMinimumPoolSize(0);
executor.setKeepAliveTime(1000 * 30);
executor.waitWhenBlocked();
executor.setThreadFactory(new DefaultThreadFactory());

If the only thread in this thread pool dies, the mininmum size of 0 prevents the generation of new threads. In an e-mail discussion Doug Lea (creator of the util.concurrent package) explained why the thread should die rather than catch any exception:

Yes, this is a documented feature, not a bug. Threads dying from
uncaught exceptions are discarded because you cannot tell if they are
in usable states.

I believe that the unexpected consequence of this in your case is that
it will only replace such threads with new ones if there are fewer
than the minimum number of threads. (See method workerDone()). If you
would like them replaced in such cases, you could increase the minimum
value.

So we think that minimum size of the thread pool should be 1 (as in the other two thread pools in the OIL2 and UIL2). Doug said about the above configuration:

I have mentioned several times to people using JBoss that setting a
minimum size of zero is usually a bad idea because it interacts badly
with other policies and settings. It cannot be disallowed, because
there ARE legitimate uses for this, but they are not common.

IMHO, genman should also review his change to catch the Throwable: Log the exception/error thrown and let the thread die. The PooledExecutor will create a new one if needed.

Is it possible to build a separate org.jboss.jms.asf package jar which contains the above fixes and can be added to the current 3.2.3 release?

Thanks in advance and regards

Joerg

[adrian.brock]

Your analysis is off.

1) Doug says we must catch all exceptions otherwise the executor is broken
(unless you prefill the pool by setting a minimum)
That is what Genman fixed, no exception is thrown from StdServerSession.run()

It is in cvs for the 3.2.4RC1. I still haven't seen what this exception actually is
and why an exception is being thrown there.

2) For the minimum size of 1, that would be unacceptable in general. If you have 100
mdbs, we cannot have 100 idle threads hanging around when there are no
messages to process. We just let the thread die if there are no messages for
30 seconds.
There is no need if set the minimum if there are no uncaught exceptions.

3) The 3.2.3 release is frozen, it will not change.

Regards,
Adrian

[thoennes]

Hello Adrian,

1) Doug says we must catch all exceptions otherwise the executor is broken
(unless you prefill the pool by setting a minimum)
That is what Genman fixed, no exception is thrown from StdServerSession.run()

But Doug also says that the thread may be in an unusable state if you any runtime exception.
Genman catches all Throwables here. This could also be a serious JVM Error which tells you that the current thread is dead.

It is in cvs for the 3.2.4RC1. I still haven't seen what this exception actually is
and why an exception is being thrown there.

It is GOOD to log the catched exception, but the thread should die anyway.

2) For the minimum size of 1, that would be unacceptable in general. If you have 100
mdbs, we cannot have 100 idle threads hanging around when there are no
messages to process. We just let the thread die if there are no messages for
30 seconds.

Agreed. But the keep alive time also works with a minimum size of 1. After 30 secs, all threads in the pool are gone.

Please review this.

Thanks, Joerg

[genman]

Could you please attempt to reproduce with the latest Jboss 3.2 branch and let us know what you find?

I do catch all throwables at this time, because I am insanely curious why the session threads have been disappearing for my application. It is usually not a good idea to catch Errors and the like, however for the time being I'm trying to get to the bottom of this.

If you can 1) tell how to reproduce the problem 2) create a unit test case which reproduces the problem 3) supply a (better) fix and send it to either Adrian or even myself, we would be inclined to apply it to the tree.

Personally, I think if Doug Lea says the threading model we have is problematic, then it probably is. In Java 1.5, the ThreadPoolExecutor is not going to allow for blocking enqueueing.

[raphael]

Doug Lea just release a newer version of the concurrent library!

http://gee.cs.oswego.edu/dl/classes/EDU/oswego/cs/dl/util/concurrent/intro.html

Here's what Doug says:

20feb2004 Version 1.3.3
PooledExecutor: Create new threads if needed when terminating. (Thanks to Bruno Dumon), and replace dying thread if it is only one.

This version should prevent JBoss from running out of executor threads (and thus stopping the processing of incoming messages) even when JBoss uses a executor.setMinimumPoolSize(0).


I'm currently in the process of upgrading from JBoss 3.0.8 to 3.2x and I don't want to wait for the next release (which hopefully contains the new cuncurrent lib version) and go through the migration process again.

So:

Can I replace the concurrent.jar file in JBoss 3.2.3 by the newer version, is there any reason which speaks against this approach?