Q: 8071326: ThreadPoolExecutor in endless thread creation loop if workQueue.take() throws RuntimeException

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

Q: 8071326: ThreadPoolExecutor in endless thread creation loop if workQueue.take() throws RuntimeException

Lev Priima
Using TPE w/ custom BlockingQueue and if RuntimeException happens in blocking BlockingQueue.take() method then this code 

new ThreadPoolExecutor(1, 1, 0, TimeUnit.NANOSECONDS, 
    new ArrayBlockingQueue<Runnable>(1) { 
        public Runnable take() throws InterruptedException { 
            throw new RuntimeException(); 
        } 
    } 
).prestartAllCoreThreads(); 

has an unbounded thread creation loop.

As a result there are many created unbounded threads in RUNNING state after printing stack trace to stderr by default UncaughtExceptionHandler. And these thread will be cleaned only when whole TPE finished.

Is this "Not an Issue"?

-- 
Lev

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

Re: Q: 8071326: ThreadPoolExecutor in endless thread creation loop if workQueue.take() throws RuntimeException

Doug Lea
On 01/27/2015 10:43 AM, Lev Priima wrote:

> Using TPE w/ custom BlockingQueue and if RuntimeException happens in blocking
> BlockingQueue.take() method then this code
>
> new ThreadPoolExecutor(1, 1, 0, TimeUnit.NANOSECONDS,
>      new ArrayBlockingQueue<Runnable>(1) {
>          public Runnable take() throws InterruptedException {
>              throw new RuntimeException();
>          }
>      }
> ).prestartAllCoreThreads();
>
> has an unbounded thread creation loop.
>
> As a result there are many created unbounded threads in RUNNING state after
> printing stack trace to stderr by default UncaughtExceptionHandler. And these
> thread will be cleaned only when whole TPE finished.
>
> Is this "Not an Issue"?
>

It is apparently an issue for someone! But I don't see any spec
violation: If a supplied BlockingQueue not only does not block but also
continuously throws exceptions, then threads using it will be
continuously replaced, given the other TPE construction arguments
here and the call to prestartAllCoreThreads.

Any attempt to change TPE to somehow guess that it should stop trying
to replace threads seems unlikely to solve any actual problem, since
the TPE remains unusable either way.

-Doug

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

Re: Q: 8071326: ThreadPoolExecutor in endless thread creation loop if workQueue.take() throws RuntimeException

Martin Buchholz-3
In reply to this post by Lev Priima


On Tue, Jan 27, 2015 at 7:43 AM, Lev Priima <[hidden email]> wrote:
And these thread will be cleaned only when whole TPE finished.


Is this really true?  Each thread should be replaced while running and so the total number of threads retained by the TPE at any one time should be no more than core pool size.
 

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

Re: Q: 8071326: ThreadPoolExecutor in endless thread creation loop if workQueue.take() throws RuntimeException

Lev Priima
Yes. And if we have BlockingQueue w/ some amount of tasks which fail with exceptions, same amount of threads(not limited by neither maximumPoolSize/corePoolSize) will hang under TPE which takes tasks from this queue.

It may cause problems if queue has a big percentage of exception-fail tasks and we eventually get OOME while unable to create new native thread.
Lev
On 01/27/2015 11:31 PM, Martin Buchholz wrote:


On Tue, Jan 27, 2015 at 7:43 AM, Lev Priima <[hidden email]> wrote:
And these thread will be cleaned only when whole TPE finished.


Is this really true?  Each thread should be replaced while running and so the total number of threads retained by the TPE at any one time should be no more than core pool size.
 


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

Re: Q: 8071326: ThreadPoolExecutor in endless thread creation loop if workQueue.take() throws RuntimeException

Lev Priima
Thanks Doug, David, Martin, especially Martin.
Is it worth to update javadoc of  ThreadPoolExecutor#Queuing section
with this caveat ?

The original confusion in custom queue implementation raise up from
javadoc, because BlockingQueue.take() interface specification does not
prohibit explicitly to throw uncaught runtime exception/errors (as any
other casual java code). But using this method in an exhaustive resource
allocation loop obliges to deal with exceptional situations in
work-producing methods more carefully.

Best Regards,
Lev

On 28.01.2015 7:17, David Holmes wrote:

> On 28/01/2015 7:03 AM, Lev Priima wrote:
>> Yes. And if we have BlockingQueue w/ some amount of tasks which fail
>> with exceptions, same amount of threads(not limited by neither
>> maximumPoolSize/corePoolSize) will hang under TPE which takes tasks from
>> this queue.
>>
>> It may cause problems if queue has a big percentage of exception-fail
>> tasks and we eventually get OOME while unable to create new native
>> thread.
>
> If you use your pathological example then of course you can get into a
> situation where the thread creation outpaces the thread termination -
> it takes time and CPU cycles for a thread to actually complete.
>
> A BlockingQueue implementation should not have an expected failure
> mode that results in regularly throwing Errors or RuntimeExceptions.
> Such a BQ implementation would need to be fixed in my opinion.
>
> The TPE is working as designed - if errors/runtime-exceptions are
> encountered the worker thread will terminate and be replaced by a
> fresh worker. If you keep feeding the worker threads such exceptions
> then you incur a high rate of thread churn. So don't do that. :)
>
> Cheers,
> David
>
>> Lev
>>
>> On 01/27/2015 11:31 PM, Martin Buchholz wrote:
>>>
>>>
>>> On Tue, Jan 27, 2015 at 7:43 AM, Lev Priima <[hidden email]
>>> <mailto:[hidden email]>> wrote:
>>>
>>>     And these thread will be cleaned only when whole TPE finished.
>>>
>>>
>>> Is this really true?  Each thread should be replaced while running and
>>> so the total number of threads retained by the TPE at any one time
>>> should be no more than core pool size.
>>

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

Re: Q: 8071326: ThreadPoolExecutor in endless thread creation loop if workQueue.take() throws RuntimeException

Martin Buchholz-3
It's hard for me to think of something we could add to the javadoc that would actually help future users enough to offset the confusion of adding subtleties of rarely encountered behavior.  I also don't see an easy way to improve the pool's reaction to exceptions coming from the queue.  Right now the reporting mechanism is the uncaught exception handler, which is under the user's control, although it is not obvious.

On Wed, Jan 28, 2015 at 4:27 AM, Lev Priima <[hidden email]> wrote:
Thanks Doug, David, Martin, especially Martin.
Is it worth to update javadoc of  ThreadPoolExecutor#Queuing section with this caveat ?

The original confusion in custom queue implementation raise up from javadoc, because BlockingQueue.take() interface specification does not prohibit explicitly to throw uncaught runtime exception/errors (as any other casual java code). But using this method in an exhaustive resource allocation loop obliges to deal with exceptional situations in work-producing methods more carefully.

Best Regards,
Lev


On 28.01.2015 7:17, David Holmes wrote:
On 28/01/2015 7:03 AM, Lev Priima wrote:
Yes. And if we have BlockingQueue w/ some amount of tasks which fail
with exceptions, same amount of threads(not limited by neither
maximumPoolSize/corePoolSize) will hang under TPE which takes tasks from
this queue.

It may cause problems if queue has a big percentage of exception-fail
tasks and we eventually get OOME while unable to create new native thread.

If you use your pathological example then of course you can get into a situation where the thread creation outpaces the thread termination - it takes time and CPU cycles for a thread to actually complete.

A BlockingQueue implementation should not have an expected failure mode that results in regularly throwing Errors or RuntimeExceptions. Such a BQ implementation would need to be fixed in my opinion.

The TPE is working as designed - if errors/runtime-exceptions are encountered the worker thread will terminate and be replaced by a fresh worker. If you keep feeding the worker threads such exceptions then you incur a high rate of thread churn. So don't do that. :)

Cheers,
David

Lev

On 01/27/2015 11:31 PM, Martin Buchholz wrote:


On Tue, Jan 27, 2015 at 7:43 AM, Lev Priima <[hidden email]
<mailto:[hidden email]>> wrote:

    And these thread will be cleaned only when whole TPE finished.


Is this really true?  Each thread should be replaced while running and
so the total number of threads retained by the TPE at any one time
should be no more than core pool size.




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

Re: Q: 8071326: ThreadPoolExecutor in endless thread creation loop if workQueue.take() throws RuntimeException

Peter Levart
On 01/28/2015 06:35 PM, Martin Buchholz wrote:
> It's hard for me to think of something we could add to the javadoc that
> would actually help future users enough to offset the confusion of adding
> subtleties of rarely encountered behavior.  I also don't see an easy way to
> improve the pool's reaction to exceptions coming from the queue.  Right now
> the reporting mechanism is the uncaught exception handler, which is under
> the user's control, although it is not obvious.

Well, there already exists these paragraphs in the class-level
ThreadPoolExecutor javadoc:

  * <dt>Hook methods</dt>
  *
  * <dd>This class provides {@code protected} overridable
  * {@link #beforeExecute(Thread, Runnable)} and
  * {@link #afterExecute(Runnable, Throwable)} methods that are called
  * before and after execution of each task.  These can be used to
  * manipulate the execution environment; for example, reinitializing
  * ThreadLocals, gathering statistics, or adding log entries.
  * Additionally, method {@link #terminated} can be overridden to perform
  * any special processing that needs to be done once the Executor has
  * fully terminated.
  *
  * <p>If hook or callback methods throw exceptions, internal worker
  * threads may in turn fail and abruptly terminate.</dd>


The last paragraph could explicitly spell-out what are the "callback"
methods. That would be enough, I think.


Regards, peter

>
> On Wed, Jan 28, 2015 at 4:27 AM, Lev Priima <[hidden email]> wrote:
>
>> Thanks Doug, David, Martin, especially Martin.
>> Is it worth to update javadoc of  ThreadPoolExecutor#Queuing section with
>> this caveat ?
>>
>> The original confusion in custom queue implementation raise up from
>> javadoc, because BlockingQueue.take() interface specification does not
>> prohibit explicitly to throw uncaught runtime exception/errors (as any
>> other casual java code). But using this method in an exhaustive resource
>> allocation loop obliges to deal with exceptional situations in
>> work-producing methods more carefully.
>>
>> Best Regards,
>> Lev
>>
>>
>> On 28.01.2015 7:17, David Holmes wrote:
>>
>>> On 28/01/2015 7:03 AM, Lev Priima wrote:
>>>
>>>> Yes. And if we have BlockingQueue w/ some amount of tasks which fail
>>>> with exceptions, same amount of threads(not limited by neither
>>>> maximumPoolSize/corePoolSize) will hang under TPE which takes tasks from
>>>> this queue.
>>>>
>>>> It may cause problems if queue has a big percentage of exception-fail
>>>> tasks and we eventually get OOME while unable to create new native
>>>> thread.
>>>>
>>> If you use your pathological example then of course you can get into a
>>> situation where the thread creation outpaces the thread termination - it
>>> takes time and CPU cycles for a thread to actually complete.
>>>
>>> A BlockingQueue implementation should not have an expected failure mode
>>> that results in regularly throwing Errors or RuntimeExceptions. Such a BQ
>>> implementation would need to be fixed in my opinion.
>>>
>>> The TPE is working as designed - if errors/runtime-exceptions are
>>> encountered the worker thread will terminate and be replaced by a fresh
>>> worker. If you keep feeding the worker threads such exceptions then you
>>> incur a high rate of thread churn. So don't do that. :)
>>>
>>> Cheers,
>>> David
>>>
>>>   Lev
>>>> On 01/27/2015 11:31 PM, Martin Buchholz wrote:
>>>>
>>>>>
>>>>> On Tue, Jan 27, 2015 at 7:43 AM, Lev Priima <[hidden email]
>>>>> <mailto:[hidden email]>> wrote:
>>>>>
>>>>>      And these thread will be cleaned only when whole TPE finished.
>>>>>
>>>>>
>>>>> Is this really true?  Each thread should be replaced while running and
>>>>> so the total number of threads retained by the TPE at any one time
>>>>> should be no more than core pool size.
>>>>>
>>>>

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

Re: Q: 8071326: ThreadPoolExecutor in endless thread creation loop if workQueue.take() throws RuntimeException

Doug Lea
On 01/29/2015 04:38 AM, Peter Levart wrote:

>   *
>   * <p>If hook or callback methods throw exceptions, internal worker
>   * threads may in turn fail and abruptly terminate.</dd>
>
>
> The last paragraph could explicitly spell-out what are the "callback" methods.
> That would be enough, I think.

Good idea; thanks. Changed to:

!  * <p>If hook, callback, or BlockingQueue methods throw exceptions,
!  * internal worker threads may in turn fail, abruptly terminate, and
!  * possibly be replaced.</dd>

-Doug

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