Finalization changes to DelegatedExecutorService

classic Classic list List threaded Threaded
10 messages Options
Reply | Threaded
Open this post in threaded view
|

Finalization changes to DelegatedExecutorService

Jason Mehrens
The changes for BugID 6399443 makes the DelegatedExecutorService invoke
shutdown when the finalizer is run.  I think there is a potential problem
where a live reference to a configurable executor service is maintained but
the unconfigurable "view" is subject to garbage collection.  In the test
case bellow, as soon as any one of the unconfigurable views is garbage
collected the live ExecutorService is shutdown.

I'm not sure if this is a real world problem because it the test case bellow
it could just as easily return the same unconfigurable view and avoid the
issue.

Regards,

Jason Mehrens


//====Test case==============
import java.util.concurrent.*;

public class TPExTest {
  private ExecutorService privatePool = Executors.newFixedThreadPool(1);

  public ExecutorService getExecutorService() {
    return Executors.unconfigurableExecutorService(privatePool);
  }

  public static void main(String[] args) {
    TPExTest test = new TPExTest();
    for(int i=Integer.MIN_VALUE; i<Integer.MAX_VALUE; i++) {
      if(test.getExecutorService().isShutdown()) {
        throw new AssertionError();
      }
      System.gc();
    }
    test.getExecutorService().shutdown();
  }
}


_______________________________________________
Concurrency-interest mailing list
[hidden email]
http://altair.cs.oswego.edu/mailman/listinfo/concurrency-interest
Reply | Threaded
Open this post in threaded view
|

Re: Finalization changes to DelegatedExecutorService

Doug Lea
Jason Mehrens wrote:

> The changes for BugID 6399443 makes the DelegatedExecutorService invoke
> shutdown when the finalizer is run.  I think there is a potential
> problem where a live reference to a configurable executor service is
> maintained but the unconfigurable "view" is subject to garbage
> collection.  In the test case bellow, as soon as any one of the
> unconfigurable views is garbage collected the live ExecutorService is
> shutdown.
>
> I'm not sure if this is a real world problem because it the test case
> bellow it could just as easily return the same unconfigurable view and
> avoid the issue.
>

Wow, thanks for the very quick scrutiny! (This change was just barely entered.)
We (JSR166 folks) have been discussing this change for a while.
If anyone else is interested, the issue is that some users
of Executors seem to believe that they should be garbage collectable when no
longer referenced even when not shutdown. This is not always a sensible
belief, but we want to help out by making more common cases automatically
finalizable, to avoid inadvertent leaks. The change here did so harmlessly for
Executors.newSingleThreadedExecutor, but as a potential compatibility
bug byproduct, also did so for any other executor wrapped within a
unconfigurableExecutorService. We'll have to change this, because it is
just barely conceivable that someone out there could be relying on this
undocumented detail. And we've sworn not to introduce any such
incompatibilities.

(The broader moral is that even when you purposely do not specify some
behaviors, people come to rely on them, so it becomes harder and harder to
change anything at all as a set of APIs become established.)

-Doug





_______________________________________________
Concurrency-interest mailing list
[hidden email]
http://altair.cs.oswego.edu/mailman/listinfo/concurrency-interest
Reply | Threaded
Open this post in threaded view
|

Re: Finalization changes to DelegatedExecutorService

Brian Goetz
> (The broader moral is that even when you purposely do not specify some
> behaviors, people come to rely on them, so it becomes harder and harder to
> change anything at all as a set of APIs become established.)

And also that no matter how hard we try, platform and performance issues
influence the design of APIs.  We restricted ourselves to the choice of
having newFixedThreadPool return ThreadPoolExecutor, or ExecutorService;
we chose the interface because of the two, it was the sensible thing to
do for a number of reasons.  But what we really wanted was some sort of
ConfigurableThreadPool interface, which we were reluctant to define
solely for this purpose because of concerns over class count, which we
were concerned about because of the need to make this stuff useful on
constrained platforms like J2ME.


_______________________________________________
Concurrency-interest mailing list
[hidden email]
http://altair.cs.oswego.edu/mailman/listinfo/concurrency-interest
Reply | Threaded
Open this post in threaded view
|

backport: problem with shutdownNow() during invokeAll()

Tom Sugden
Hi,

I'm having some difficulty with the shutdownNow functionality of the cached
thread pool executor service. The scenario I am trying to test is this:

   - submit a number of tasks for execution using the invokeAll() method
   - wait until some of the tasks have definitely begun execution
   - invoke shutdownNow() on the executor service

I would expect this to:

   - prevent any tasks that have yet to be called from ever being called
   - interrupt any tasks that have already been called

In a loop of 100 iterations, my program regularly stops in a deadlock. I'm
quite new to these concurrency utilities, so there's a fair chance I'm doing
something stupid! It would be a great help if somebody could cast an eye
over my code below and offer advice.

The strangest thing is this: if the lines marked "*** A ***" and *** B ***"
are commented out, the program runs fine over thousands of iterations. This
suggests (but doesn't prove) that the deadlock arises if one of the tasks
has already completed before the shutdownNow() invocation.

Please note that I am using the Java 1.4 backport of the concurrency
utilities.

Cheers,
Tom Sugden

---

package concurrency.test;

import java.util.ArrayList;
import java.util.List;

import edu.emory.mathcs.backport.java.util.concurrent.Callable;
import edu.emory.mathcs.backport.java.util.concurrent.ExecutorService;
import edu.emory.mathcs.backport.java.util.concurrent.Executors;
import edu.emory.mathcs.backport.java.util.concurrent.TimeUnit;

/**
 * Submits a list of callable tasks to an executor service, then terminates
the
 * executor service at an intermediate stage of processing.
 */
public class BlockingTaskExecutorTest
{
    public static void main(String[] args) throws InterruptedException
    {
        for (int i = 1; i <= 100; i++)
        {
            System.out.println("Iteration: " + i);
           
            final ExecutorService executor =
Executors.newCachedThreadPool();

            final NotificationReceiver notifier1 = new
NotificationReceiver();
            final NotificationReceiver notifier2 = new
NotificationReceiver();

            final Callable task1 = new BlockingTask(notifier1);
            final Callable task2 = new BlockingTask(notifier2);
            final Callable task3 = new NonBlockingTask();        // *** A
***

            final List tasks = new ArrayList();
            tasks.add(task1);
            tasks.add(task2);
            tasks.add(task3);                                    // *** B
***

            // start a thread to invoke the tasks
            Thread thread = new Thread()
            {
                public void run()
                {
                    try
                    {
                        executor.invokeAll(tasks);
                    }
                    catch (Throwable e)
                    {
                        e.printStackTrace();
                    }
                }
            };
            thread.start();
           
            // wait until tasks begin execution
            notifier1.waitForNotification();
            notifier2.waitForNotification();

            // now try to shutdown the executor service while tasks are
blocked.
            // This should cause the tasks to be interupted.
            executor.shutdownNow();
            boolean stopped = executor.awaitTermination(5,
TimeUnit.SECONDS);
            System.out.println("Terminated? " + stopped);

            // wait for the invocation thread to complete
            thread.join();
        }
    }
}

/**
 * A helper class with a method to wait for a notification. The notification
is
 * received via the <code>sendNotification</code> method.
 */
class NotificationReceiver
{
    /** Has the notifier been notified? */
    boolean mNotified = false;

    /**
     * Notify the notification receiver.
     */
    public synchronized void sendNotification()
    {
        mNotified = true;
        notifyAll();
    }

    /**
     * Waits until a notification has been received.
     *
     * @throws InterruptedException
     *             if the wait is interrupted
     */
    public synchronized void waitForNotification() throws
InterruptedException
    {
        while (!mNotified)
        {
            wait();
        }
    }
}

/**
 * A callable task that blocks until it is interupted. This task sends a
 * notification to a notification receiver when it is first called.
 */
class BlockingTask implements Callable
{
    private final NotificationReceiver mReceiver;
   
    BlockingTask(NotificationReceiver notifier)
    {
        mReceiver = notifier;
    }

    /*
     * (non-Javadoc)
     * @see edu.emory.mathcs.backport.java.util.concurrent.Callable#call()
     */
    public Object call() throws Exception
    {
        mReceiver.sendNotification();
       
        // wait indefinitely until task is interupted
        while (true)
        {
            synchronized (this)
            {
                wait();
            }
        }
    }
}

/**
 * A callable task that simply returns a string result.
 */
class NonBlockingTask implements Callable
{
    /*
     * (non-Javadoc)
     * @see edu.emory.mathcs.backport.java.util.concurrent.Callable#call()
     */
    public Object call() throws Exception
    {
        return "NonBlockingTaskResult";
    }
}

_______________________________________________
Concurrency-interest mailing list
[hidden email]
http://altair.cs.oswego.edu/mailman/listinfo/concurrency-interest
Reply | Threaded
Open this post in threaded view
|

Re: backport: problem with shutdownNow() during invokeAll()

tpeierls
A task that has been executed with executor.execute(task) but has not yet run internally might not see the interruption sent by shutdownNow, in which case Future.get() for this task can block indefinitely. I think that is what is happening with your NonBlockingTask; it hasn't had a chance to run by the time shutdownNow interrupts the thread it is scheduled to run on, so that when invokeAll tries to get its value, it blocks forever.

The upshot is that you shouldn't mix shutdownNow with invokeAll. Consider using timed InvokeAll, or if you only need one result, invokeAny. That way you can reuse the executor over multiple iterations.

This is true for all j.u.c flavors, not just the backport. I reproduced the behavior using a recent Mustang build.

--tim

On 4/3/06, Tom Sugden <[hidden email]> wrote:
Hi,

I'm having some difficulty with the shutdownNow functionality of the cached
thread pool executor service. The scenario I am trying to test is this:

   - submit a number of tasks for execution using the invokeAll() method
   - wait until some of the tasks have definitely begun execution
   - invoke shutdownNow() on the executor service

I would expect this to:

   - prevent any tasks that have yet to be called from ever being called
   - interrupt any tasks that have already been called

In a loop of 100 iterations, my program regularly stops in a deadlock. I'm
quite new to these concurrency utilities, so there's a fair chance I'm doing
something stupid! It would be a great help if somebody could cast an eye
over my code below and offer advice.

The strangest thing is this: if the lines marked "*** A ***" and *** B ***"
are commented out, the program runs fine over thousands of iterations. This
suggests (but doesn't prove) that the deadlock arises if one of the tasks
has already completed before the shutdownNow() invocation.

Please note that I am using the Java 1.4 backport of the concurrency
utilities.

Cheers,
Tom Sugden

---

package concurrency.test;

import java.util.ArrayList;
import java.util.List;

import edu.emory.mathcs.backport.java.util.concurrent.Callable;
import edu.emory.mathcs.backport.java.util.concurrent.ExecutorService ;
import edu.emory.mathcs.backport.java.util.concurrent.Executors;
import edu.emory.mathcs.backport.java.util.concurrent.TimeUnit;

/**
* Submits a list of callable tasks to an executor service, then terminates
the
* executor service at an intermediate stage of processing.
*/
public class BlockingTaskExecutorTest
{
    public static void main(String[] args) throws InterruptedException
    {
        for (int i = 1; i <= 100; i++)
        {
            System.out.println("Iteration: " + i);

            final ExecutorService executor =
Executors.newCachedThreadPool();

            final NotificationReceiver notifier1 = new
NotificationReceiver();
            final NotificationReceiver notifier2 = new
NotificationReceiver();

            final Callable task1 = new BlockingTask(notifier1);
            final Callable task2 = new BlockingTask(notifier2);
            final Callable task3 = new NonBlockingTask();        // *** A
***

            final List tasks = new ArrayList();
            tasks.add(task1);
            tasks.add(task2);
             tasks.add(task3);                                    // *** B
***

            // start a thread to invoke the tasks
            Thread thread = new Thread()
            {
                public void run()
                {
                    try
                    {
                        executor.invokeAll(tasks);
                    }
                    catch (Throwable e)
                    {
                        e.printStackTrace();
                    }
                }
            };
            thread.start();

            // wait until tasks begin execution
            notifier1.waitForNotification ();
            notifier2.waitForNotification();

            // now try to shutdown the executor service while tasks are
blocked.
            // This should cause the tasks to be interupted.
             executor.shutdownNow();
            boolean stopped = executor.awaitTermination(5,
TimeUnit.SECONDS);
            System.out.println("Terminated? " + stopped);

            // wait for the invocation thread to complete
            thread.join();
        }
    }
}

/**
* A helper class with a method to wait for a notification. The notification
is
* received via the <code>sendNotification</code> method.
*/
class NotificationReceiver
{
    /** Has the notifier been notified? */
    boolean mNotified = false;

    /**
     * Notify the notification receiver.
     */
    public synchronized void sendNotification()
    {
        mNotified = true;
        notifyAll();
    }

    /**
     * Waits until a notification has been received.
     *
     * @throws InterruptedException
     *             if the wait is interrupted
     */
    public synchronized void waitForNotification() throws
InterruptedException
    {
        while (!mNotified)
        {
            wait();
        }
    }
}

/**
* A callable task that blocks until it is interupted. This task sends a
* notification to a notification receiver when it is first called.
*/
class BlockingTask implements Callable
{
    private final NotificationReceiver mReceiver;

    BlockingTask(NotificationReceiver notifier)
    {
        mReceiver = notifier;
    }

    /*
     * (non-Javadoc)
     * @see edu.emory.mathcs.backport.java.util.concurrent.Callable#call()
     */
    public Object call() throws Exception
    {
        mReceiver.sendNotification();

        // wait indefinitely until task is interupted
        while (true)
        {
            synchronized (this)
            {
                wait();
            }
        }
    }
}

/**
* A callable task that simply returns a string result.
*/
class NonBlockingTask implements Callable
{
    /*
     * (non-Javadoc)
     * @see edu.emory.mathcs.backport.java.util.concurrent.Callable#call()
     */
    public Object call() throws Exception
    {
        return "NonBlockingTaskResult";
    }
}

_______________________________________________
Concurrency-interest mailing list
[hidden email]
<a href="http://altair.cs.oswego.edu/mailman/listinfo/concurrency-interest" target="_blank" onclick="return top.js.OpenExtLink(window,event,this)"> http://altair.cs.oswego.edu/mailman/listinfo/concurrency-interest


_______________________________________________
Concurrency-interest mailing list
[hidden email]
http://altair.cs.oswego.edu/mailman/listinfo/concurrency-interest
Reply | Threaded
Open this post in threaded view
|

Re: backport: problem with shutdownNow() during invokeAll()

Joe Bowbeer
In reply to this post by Tom Sugden
Adding to Tim's comment:

Note that shutdownNow returns a list of tasks that were still on the
queue, uncompleted and uninterrupted.


On 4/3/06, Tom Sugden <[hidden email]> wrote:

> Hi,
>
> I'm having some difficulty with the shutdownNow functionality of the cached
> thread pool executor service. The scenario I am trying to test is this:
>
>    - submit a number of tasks for execution using the invokeAll() method
>    - wait until some of the tasks have definitely begun execution
>    - invoke shutdownNow() on the executor service
>
> I would expect this to:
>
>    - prevent any tasks that have yet to be called from ever being called
>    - interrupt any tasks that have already been called
>

_______________________________________________
Concurrency-interest mailing list
[hidden email]
http://altair.cs.oswego.edu/mailman/listinfo/concurrency-interest
Reply | Threaded
Open this post in threaded view
|

Re: backport: problem with shutdownNow() during invokeAll()

Bart Jacobs
In reply to this post by tpeierls
I don't know if there are any use cases for this, but perhaps there
should be an ExecutorTask interface, defined as follows:

public interface ExecutorTask extends Runnable {
    public void run();
    public void cancel();
}

This would be used by someone submitting a task instead of Runnable if
they want to be notified if it turns out that the task they submitted
will not be run after all. It would be the responsibility of whoever
calls shutdownNow() to walk the returned collection and call cancel() if
applicable:

for (Runnable task : executor.shutdownNow()) {
    if (task instanceof ExecutorTask)
        ((ExecutorTask)task).cancel();
}

FutureTask would implement ExecutorTask and cancel itself when getting a
cancel() call. This would enable the scenario below.

Best,-
Bart Jacobs

Tim Peierls schreef:

> A task that has been executed with executor.execute(task) but has not
> yet run internally might not see the interruption sent by shutdownNow,
> in which case Future.get() for this task can block indefinitely. I
> think that is what is happening with your NonBlockingTask; it hasn't
> had a chance to run by the time shutdownNow interrupts the thread it
> is scheduled to run on, so that when invokeAll tries to get its value,
> it blocks forever.
>
> The upshot is that you shouldn't mix shutdownNow with invokeAll.
> Consider using timed InvokeAll, or if you only need one result,
> invokeAny. That way you can reuse the executor over multiple iterations.
>
> This is true for all j.u.c flavors, not just the backport. I
> reproduced the behavior using a recent Mustang build.
>
> --tim
>
> On 4/3/06, *Tom Sugden* < [hidden email]
> <mailto:[hidden email]>> wrote:
>
>     Hi,
>
>     I'm having some difficulty with the shutdownNow functionality of
>     the cached
>     thread pool executor service. The scenario I am trying to test is
>     this:
>
>        - submit a number of tasks for execution using the invokeAll()
>     method
>        - wait until some of the tasks have definitely begun execution
>        - invoke shutdownNow() on the executor service
>
>     I would expect this to:
>
>        - prevent any tasks that have yet to be called from ever being
>     called
>        - interrupt any tasks that have already been called
>
>     In a loop of 100 iterations, my program regularly stops in a
>     deadlock. I'm
>     quite new to these concurrency utilities, so there's a fair chance
>     I'm doing
>     something stupid! It would be a great help if somebody could cast
>     an eye
>     over my code below and offer advice.
>
>     The strangest thing is this: if the lines marked "*** A ***" and
>     *** B ***"
>     are commented out, the program runs fine over thousands of
>     iterations. This
>     suggests (but doesn't prove) that the deadlock arises if one of
>     the tasks
>     has already completed before the shutdownNow() invocation.
>
>     Please note that I am using the Java 1.4 backport of the concurrency
>     utilities.
>
>     Cheers,
>     Tom Sugden
>
>     ---
>
>     package concurrency.test;
>
>     import java.util.ArrayList;
>     import java.util.List;
>
>     import edu.emory.mathcs.backport.java.util.concurrent.Callable;
>     import
>     edu.emory.mathcs.backport.java.util.concurrent.ExecutorService ;
>     import edu.emory.mathcs.backport.java.util.concurrent.Executors;
>     import edu.emory.mathcs.backport.java.util.concurrent.TimeUnit;
>
>     /**
>     * Submits a list of callable tasks to an executor service, then
>     terminates
>     the
>     * executor service at an intermediate stage of processing.
>     */
>     public class BlockingTaskExecutorTest
>     {
>         public static void main(String[] args) throws InterruptedException
>         {
>             for (int i = 1; i <= 100; i++)
>             {
>                 System.out.println("Iteration: " + i);
>
>                 final ExecutorService executor =
>     Executors.newCachedThreadPool();
>
>                 final NotificationReceiver notifier1 = new
>     NotificationReceiver();
>                 final NotificationReceiver notifier2 = new
>     NotificationReceiver();
>
>                 final Callable task1 = new BlockingTask(notifier1);
>                 final Callable task2 = new BlockingTask(notifier2);
>                 final Callable task3 = new
>     NonBlockingTask();        // *** A
>     ***
>
>                 final List tasks = new ArrayList();
>                 tasks.add(task1);
>                 tasks.add(task2);
>                
>     tasks.add(task3);                                    // *** B
>     ***
>
>                 // start a thread to invoke the tasks
>                 Thread thread = new Thread()
>                 {
>                     public void run()
>                     {
>                         try
>                         {
>                             executor.invokeAll(tasks);
>                         }
>                         catch (Throwable e)
>                         {
>                             e.printStackTrace();
>                         }
>                     }
>                 };
>                 thread.start();
>
>                 // wait until tasks begin execution
>                 notifier1.waitForNotification ();
>                 notifier2.waitForNotification();
>
>                 // now try to shutdown the executor service while
>     tasks are
>     blocked.
>                 // This should cause the tasks to be interupted.
>                  executor.shutdownNow();
>                 boolean stopped = executor.awaitTermination(5,
>     TimeUnit.SECONDS);
>                 System.out.println("Terminated? " + stopped);
>
>                 // wait for the invocation thread to complete
>                 thread.join();
>             }
>         }
>     }
>
>     /**
>     * A helper class with a method to wait for a notification. The
>     notification
>     is
>     * received via the <code>sendNotification</code> method.
>     */
>     class NotificationReceiver
>     {
>         /** Has the notifier been notified? */
>         boolean mNotified = false;
>
>         /**
>          * Notify the notification receiver.
>          */
>         public synchronized void sendNotification()
>         {
>             mNotified = true;
>             notifyAll();
>         }
>
>         /**
>          * Waits until a notification has been received.
>          *
>          * @throws InterruptedException
>          *             if the wait is interrupted
>          */
>         public synchronized void waitForNotification() throws
>     InterruptedException
>         {
>             while (!mNotified)
>             {
>                 wait();
>             }
>         }
>     }
>
>     /**
>     * A callable task that blocks until it is interupted. This task
>     sends a
>     * notification to a notification receiver when it is first called.
>     */
>     class BlockingTask implements Callable
>     {
>         private final NotificationReceiver mReceiver;
>
>         BlockingTask(NotificationReceiver notifier)
>         {
>             mReceiver = notifier;
>         }
>
>         /*
>          * (non-Javadoc)
>          * @see
>     edu.emory.mathcs.backport.java.util.concurrent.Callable#call()
>          */
>         public Object call() throws Exception
>         {
>             mReceiver.sendNotification();
>
>             // wait indefinitely until task is interupted
>             while (true)
>             {
>                 synchronized (this)
>                 {
>                     wait();
>                 }
>             }
>         }
>     }
>
>     /**
>     * A callable task that simply returns a string result.
>     */
>     class NonBlockingTask implements Callable
>     {
>         /*
>          * (non-Javadoc)
>          * @see
>     edu.emory.mathcs.backport.java.util.concurrent.Callable#call()
>          */
>         public Object call() throws Exception
>         {
>             return "NonBlockingTaskResult";
>         }
>     }
>
>     _______________________________________________
>     Concurrency-interest mailing list
>     [hidden email]
>     <mailto:[hidden email]>
>     http://altair.cs.oswego.edu/mailman/listinfo/concurrency-interest
>
>
> ------------------------------------------------------------------------
>
> _______________________________________________
> Concurrency-interest mailing list
> [hidden email]
> http://altair.cs.oswego.edu/mailman/listinfo/concurrency-interest
>  
_______________________________________________
Concurrency-interest mailing list
[hidden email]
http://altair.cs.oswego.edu/mailman/listinfo/concurrency-interest
Reply | Threaded
Open this post in threaded view
|

Re: backport: problem with shutdownNow() during invokeAll()

Joe Bowbeer
Interesting idea.  Given that ExecutorTask interface would be so close
to FutureTask and FutureTask is so flexible already, would
ExecutorTask really carry its weight?

FWIW, I always create my own tasks directly, using FutureTask
constructor (or my own subclass), unless I'm doing something very
simple without any corner cases.  (Well, as I said, I always create my
own tasks using FutureTask before executing...)

This also avoids having to map the tasks returned from shutdownNow
back to the tasks submitted or invoked.  It also allows me to track
the tracks through the process starting from before they were
submitted for execution.


Note however that my original comment may have been a red herring.

As Tim pointed out to me off-list:

In Tom's case, the queue is always empty because he uses
newCachedThreadPool with its internal SynchronousQueue.


On 4/3/06, Bart Jacobs <[hidden email]> wrote:

> I don't know if there are any use cases for this, but perhaps there
> should be an ExecutorTask interface, defined as follows:
>
> public interface ExecutorTask extends Runnable {
>     public void run();
>     public void cancel();
> }
>
> This would be used by someone submitting a task instead of Runnable if
> they want to be notified if it turns out that the task they submitted
> will not be run after all. It would be the responsibility of whoever
> calls shutdownNow() to walk the returned collection and call cancel() if
> applicable:
>
> for (Runnable task : executor.shutdownNow()) {
>     if (task instanceof ExecutorTask)
>         ((ExecutorTask)task).cancel();
> }
>
> FutureTask would implement ExecutorTask and cancel itself when getting a
> cancel() call. This would enable the scenario below.
>
> Best,-
> Bart Jacobs
>
> Tim Peierls schreef:
> > A task that has been executed with executor.execute(task) but has not
> > yet run internally might not see the interruption sent by shutdownNow,
> > in which case Future.get() for this task can block indefinitely. I
> > think that is what is happening with your NonBlockingTask; it hasn't
> > had a chance to run by the time shutdownNow interrupts the thread it
> > is scheduled to run on, so that when invokeAll tries to get its value,
> > it blocks forever.
> >
> > The upshot is that you shouldn't mix shutdownNow with invokeAll.
> > Consider using timed InvokeAll, or if you only need one result,
> > invokeAny. That way you can reuse the executor over multiple iterations.
> >
> > This is true for all j.u.c flavors, not just the backport. I
> > reproduced the behavior using a recent Mustang build.
> >
> > --tim
> >

_______________________________________________
Concurrency-interest mailing list
[hidden email]
http://altair.cs.oswego.edu/mailman/listinfo/concurrency-interest
Reply | Threaded
Open this post in threaded view
|

Re: backport: problem with shutdownNow() during invokeAll()

tpeierls
In reply to this post by Joe Bowbeer
For newCachedThreadPool(), which uses a SynchronousQueue, the list returned by shutdownNow is always empty, so you might think that all submitted tasks either complete or are cancelled by shutdownNow.

But a Runnable can be accepted by a worker thread and never run. shutdownNow tries to cancel all running tasks by interrupting all worker threads, and a newly-accepted runnable might be abandoned without ever running. I don't know of an easy way to detect this situation, so try to design things in such a way that it doesn't matter if it happens.

(Shameless plug: There is a nice way to collect the list of tasks that were started and cancelled around the time of shutdownNow; see Section 7.2.5 of the upcoming book Java Concurrency in Practice.)

I still think the best advice is to avoid using shutdownNow as a way to cancel a group of tasks. Consider using the invokeAll/invokeAny methods, thereby allowing the pool to be reused.

--tim

On 4/3/06, Joe Bowbeer <[hidden email]> wrote:
Adding to Tim's comment:

Note that shutdownNow returns a list of tasks that were still on the
queue, uncompleted and uninterrupted.


On 4/3/06, Tom Sugden <[hidden email]> wrote:

> Hi,
>
> I'm having some difficulty with the shutdownNow functionality of the cached
> thread pool executor service. The scenario I am trying to test is this:
>
>    - submit a number of tasks for execution using the invokeAll() method
>    - wait until some of the tasks have definitely begun execution
>    - invoke shutdownNow() on the executor service
>
> I would expect this to:
>
>    - prevent any tasks that have yet to be called from ever being called
>    - interrupt any tasks that have already been called


_______________________________________________
Concurrency-interest mailing list
[hidden email]
http://altair.cs.oswego.edu/mailman/listinfo/concurrency-interest
Reply | Threaded
Open this post in threaded view
|

RE: backport: problem with shutdownNow() during invokeAll()

Tom Sugden
Hi,

Thanks for all the responses to my question. They were very useful and
taught me some more about the concurrency utilities. I've redeveloped my
code so that:

   - the ExecutorService is reused and shutdownNow() is not used for
cancellation purposes
   - tasks are submitted as anonymous FutureTasks, allowing me to
     override done() to invoke a callback
   - tasks are cancelled via the FutureTask#cancel(boolean) method

Everything seems to be working nicely. I did notice one other piece of
surprising behaviour during my experiments, but will post another email
about it, just in case it is unintended.

Thanks again,
Tom

-----Original Message-----
From: [hidden email] [mailto:[hidden email]] On Behalf Of Tim
Peierls
Sent: 03 April 2006 22:58
To: Tom Sugden; Joe Bowbeer
Cc: [hidden email]
Subject: Re: [concurrency-interest] backport: problem with shutdownNow()
during invokeAll()

For newCachedThreadPool(), which uses a SynchronousQueue, the list returned
by shutdownNow is always empty, so you might think that all submitted tasks
either complete or are cancelled by shutdownNow.

But a Runnable can be accepted by a worker thread and never run. shutdownNow
tries to cancel all running tasks by interrupting all worker threads, and a
newly-accepted runnable might be abandoned without ever running. I don't
know of an easy way to detect this situation, so try to design things in
such a way that it doesn't matter if it happens.

(Shameless plug: There is a nice way to collect the list of tasks that were
started and cancelled around the time of shutdownNow; see Section 7.2.5 of
the upcoming book Java Concurrency in Practice.)

I still think the best advice is to avoid using shutdownNow as a way to
cancel a group of tasks. Consider using the invokeAll/invokeAny methods,
thereby allowing the pool to be reused.

--tim

On 4/3/06, Joe Bowbeer <[hidden email]> wrote:

>
> Adding to Tim's comment:
>
> Note that shutdownNow returns a list of tasks that were still on the
> queue, uncompleted and uninterrupted.
>
>
> On 4/3/06, Tom Sugden <[hidden email]> wrote:
> > Hi,
> >
> > I'm having some difficulty with the shutdownNow functionality of the
> cached
> > thread pool executor service. The scenario I am trying to test is this:
> >
> >    - submit a number of tasks for execution using the invokeAll() method
> >    - wait until some of the tasks have definitely begun execution
> >    - invoke shutdownNow() on the executor service
> >
> > I would expect this to:
> >
> >    - prevent any tasks that have yet to be called from ever being called
> >    - interrupt any tasks that have already been called
>

_______________________________________________
Concurrency-interest mailing list
[hidden email]
http://altair.cs.oswego.edu/mailman/listinfo/concurrency-interest