Salut,
Bongjae Chang wrote:
> Hi,
>
> I know current DefaultThreadPool is modified from PipelineThreadPool and
> original DefaultThreadPool is renamed to JDKThreadPool at Rev.3265.
>
> But I think that some logic needs more.
>
> Original DefaultThreadPool had beforeExecute() and afterExecute() like this.
>
> ---
> protected void beforeExecute(Thread t, Runnable r) {
> ((WorkerThreadImpl) t).createByteBuffer(false);
> }
>
> protected void afterExecute(Runnable r, Throwable t ) {
> ((WorkerThreadImpl) Thread.currentThread()).reset();
> }
> ---
>
> But current DefaultThreadPool doesn't have them.
>
> So sometimes I could see the following error when a message was being
> parsed.
>
> ---
> java.lang.NullPointerException
> at
> com.sun.enterprise.mgmt.transport.grizzly.GrizzlyMessageProtocolParser.startBuffer(*GrizzlyMessageProtocolParser.java:158*)
> at
> com.sun.grizzly.filter.ParserProtocolFilter.execute(ParserProtocolFilter.java:134)
> at
> com.sun.grizzly.DefaultProtocolChain.executeProtocolFilter(DefaultProtocolChain.java:135)
> at
> com.sun.grizzly.DefaultProtocolChain.execute(DefaultProtocolChain.java:102)
> at
> com.sun.grizzly.DefaultProtocolChain.execute(DefaultProtocolChain.java:88)
> at
> com.sun.grizzly.DefaultCallbackHandler.onRead(DefaultCallbackHandler.java:149)
> at
> com.sun.grizzly.DefaultCallbackHandler.onWrite(DefaultCallbackHandler.java:163)
> at
> com.sun.grizzly.CallbackHandlerContextTask.doCall(CallbackHandlerContextTask.java:66)
> at
> com.sun.grizzly.SelectionKeyContextTask.call(SelectionKeyContextTask.java:57)
> at com.sun.grizzly.ContextTask.run(ContextTask.java:69)
> at
> com.sun.grizzly.util.FixedThreadPool$BasicWorker.dowork(FixedThreadPool.java:338)
> at
> com.sun.grizzly.util.FixedThreadPool$BasicWorker.run(FixedThreadPool.java:321)
> at java.lang.Thread.run(Thread.java:619)
> ---
>
> Here is GrizzlyMessageProtocolParser.java's 158 line.
> ---
> public void startBuffer( ByteBuffer bb ) {
> this.byteBuffer = bb;
> *if( byteBuffer.capacity() <
> MessageImpl.MAX_TOTAL_MESSAGE_LENGTH ) { // 158 line*
> if( LOG.isLoggable( Level.WARNING ) )
> LOG.log( Level.WARNING, "byte buffer capacity is too
> small: capacity = " + byteBuffer.capacity() );
> }
> }
> ---
>
> It seems that ByteBuffer is null. As you know,
> ProtocolParser#startBuffer(ByteBuffer bb) is called by
> ParserProtocolFilter#execute().
>
> The ByteBuffer is same to WorkerThread's byteBuffer.
>
> I think that this may be a side-effect between rev.3265 and Grizzly
> issue #605(https://grizzly.dev.java.net/issues/show_bug.cgi?id=605, "
> Eliminating the byte buffer's allocation in WorkerThread#detach()")
>
> So I think that it is better that FixedThreadPool supports
> beforeExecute() and afterExecute() like JDK's thread pool.
>
> What do you think?
Agree. I've just tried to reproduce the issue on Ubuntu without sucess.
Which OS are you using? In any case, go ahead and commit!
Thanks!
-- Jeanfrancois
>
> I attached the proposed patch.
>
> Thanks.
>
>
>
>
>
> Index: main/java/com/sun/grizzly/util/DefaultThreadPool.java
> ===================================================================
> --- main/java/com/sun/grizzly/util/DefaultThreadPool.java (revision 3293)
> +++ main/java/com/sun/grizzly/util/DefaultThreadPool.java (working copy)
> @@ -343,6 +343,16 @@
> sb.append(", is-shutdown=").append(isShutdown());
> }
>
> + @Override
> + protected void beforeExecute(Thread t, Runnable r) {
> + ((WorkerThreadImpl) t).createByteBuffer(false);
> + }
> +
> + @Override
> + protected void afterExecute(Runnable r, Throwable t) {
> + ((WorkerThreadImpl) Thread.currentThread()).reset();
> + }
> +
> private class DefaultWorkerThreadFactory implements ThreadFactory {
> public Thread newThread(Runnable r) {
> Thread thread = new WorkerThreadImpl(DefaultThreadPool.this,
> Index: main/java/com/sun/grizzly/util/FixedThreadPool.java
> ===================================================================
> --- main/java/com/sun/grizzly/util/FixedThreadPool.java (revision 3293)
> +++ main/java/com/sun/grizzly/util/FixedThreadPool.java (working copy)
> @@ -309,7 +309,46 @@
> return threadFactory;
> }
>
> -
> + /**
> + * Method invoked prior to executing the given Runnable in the
> + * given thread. This method is invoked by thread <tt>t</tt> that
> + * will execute task <tt>r</tt>, and may be used to re-initialize
> + * ThreadLocals, or to perform logging.
> + *
> + * <p>This implementation does nothing, but may be customized in
> + * subclasses. Note: To properly nest multiple overridings, subclasses
> + * should generally invoke <tt>super.beforeExecute</tt> at the end of
> + * this method.
> + *
> + * @param t the thread that will run task r.
> + * @param r the task that will be executed.
> + */
> + protected void beforeExecute(Thread t, Runnable r) { }
> +
> + /**
> + * Method invoked upon completion of execution of the given Runnable.
> + * This method is invoked by the thread that executed the task. If
> + * non-null, the Throwable is the uncaught <tt>RuntimeException</tt>
> + * or <tt>Error</tt> that caused execution to terminate abruptly.
> + *
> + * <p><b>Note:</b> When actions are enclosed in tasks (such as
> + * {_at_link <mailto:{_at_link> java.util.concurrent.FutureTask}) either
> explicitly or via methods such as
> + * <tt>submit</tt>, these task objects catch and maintain
> + * computational exceptions, and so they do not cause abrupt
> + * termination, and the internal exceptions are <em>not</em>
> + * passed to this method.
> + *
> + * <p>This implementation does nothing, but may be customized in
> + * subclasses. Note: To properly nest multiple overridings, subclasses
> + * should generally invoke <tt>super.afterExecute</tt> at the
> + * beginning of this method.
> + *
> + * @param r the runnable that has completed.
> + * @param t the exception that caused termination, or null if
> + * execution completed normally.
> + */
> + protected void afterExecute(Runnable r, Throwable t) { }
> +
> protected class BasicWorker implements Runnable{
> Thread t;
>
> @@ -333,10 +372,15 @@
> if (r == poison || r == null){
> return;
> }
> + boolean ran = false;
> + beforeExecute( t, r );
> try {
> approximateRunningWorkerCount.incrementAndGet();
> r.run();
> + afterExecute( r, null );
> } catch( Throwable t ) {
> + if (!ran)
> + afterExecute( r, t );
> throw t;
> } finally {
> approximateRunningWorkerCount.decrementAndGet();
>
> --
> Bongjae Chang
>
>
> ------------------------------------------------------------------------
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe_at_grizzly.dev.java.net
> For additional commands, e-mail: dev-help_at_grizzly.dev.java.net