On Fri, Jul 4, 2008 at 6:33 AM, Jerome Louvel <[EMAIL PROTECTED]> wrote:
> Tim, instead of wrapping the submitted tasks with thread-local copy/reset
> code, I've chosen to use the ability to provide a ThreadFactory to the
> Executors static creation method.
>
> Do you see any drawback compared to your suggested approach?
>
Yes. :-(
The run method of a thread returned by ThreadFactory is not called once per
task, but only once when the thread is first added to the pool. So pool
thread A will "burn in" the current values of whatever thread is current
when thread A is added to the pool. But thread A in general is used to
handle multiple tasks, each with different current Applications, Contexts,
VirtualHosts, and Responses, for which the burned in values will be
incorrect.
I recommend replacing Context.getExecutorService() with
Context.wrapExecutorService(ExecutorService exec) as follows:
public ExecutorService wrapExecutorService(final ExecutorService exec) {
return new AbstractExecutorService() {
public void execute(final Runnable runnable) {
final Application currentApplication =
Application.getCurrent();
final Context currentContext = Context.getCurrent();
final Integer currentVirtualHost = VirtualHost.getCurrent();
final Response currentResponse = Response.getCurrent();
execute(new Runnable() {
public void run() {
// Copy the thread local variables
Response.setCurrent(currentResponse);
Context.setCurrent(currentContext);
VirtualHost.setCurrent(currentVirtualHost);
Application.setCurrent(currentApplication);
try {
// Run the user task
runnable.run();
} finally {
// Reset the thread local variables
Response.setCurrent(null);
Context.setCurrent(null);
VirtualHost.setCurrent(-1);
Application.setCurrent(null);
}
}
});
}
public boolean isShutdown() { return exec.isShutdown(); }
public boolean isTerminated() { return exec.isTerminated(); }
public void shutdown() { exec.shutdown(); }
public List<Runnable> shutdownNow() { return exec.shutdownNow();
}
public boolean awaitTermination(long timeout, TimeUnit unit)
throws InterruptedException {
return exec.awaitTermination(timeout, unit);
}
};
}
If you really want to keep Context.getExecutorService, change
createExecutorService to return
wrapExecutorService(Executors.newCachedThreadPool());
--tim
