RFR JDK-7153400: ThreadPoolExecutor's setCorePoolSize method allows corePoolSize > maxPoolSize

[Martin Buchholz]

Pavel,

Thanks for writing a test.

We (jsr166 maintainers will add the jtreg test to jsr166 CVS when it has
passed review.

Instead of "succeed", I would just write main-line code.  If you want
per-api-call granularity, write a testng test.

Instead of "fail", I suggest as in jsr166 CVS
src/test/tck/JSR166TestCase.java :

    public void assertThrows(Class<? extends Throwable>
expectedExceptionClass,
                             Runnable... throwingActions) {
        for (Runnable throwingAction : throwingActions) {
            boolean threw = false;
            try { throwingAction.run(); }
            catch (Throwable t) {
                threw = true;
                if (!expectedExceptionClass.isInstance(t)) {
                    AssertionFailedError afe =
                        new AssertionFailedError
                        ("Expected " + expectedExceptionClass.getName() +
                         ", got " + t.getClass().getName());
                    afe.initCause(t);
                    threadUnexpectedException(afe);
                }
            }
            if (!threw)
                shouldThrow(expectedExceptionClass.getName());
        }
    }

I suggest checking the return from p.awaitTermination
            p.awaitTermination(1, TimeUnit.MINUTES);

as in src/test/tck/JSR166TestCase.java:


    /**
     * Waits out termination of a thread pool or fails doing so.
     */
    void joinPool(ExecutorService exec) {
        try {
            exec.shutdown();
            assertTrue("ExecutorService did not terminate in a timely
manner",
                       exec.awaitTermination(2 * LONG_DELAY_MS,
MILLISECONDS));
        } catch (SecurityException ok) {
            // Allowed in case test doesn't have privs
        } catch (InterruptedException ie) {
            fail("Unexpected InterruptedException");
        }
    }




On Wed, May 14, 2014 at 10:03 AM, Mike Duigou <mike.duigou at oracle.com>wrote:

> Hi Pavel;
>
> The change and test looks good. Will the test be upstreamed or will Doug
> be adding a similar test in his upstream?
>
> Mike
>
> On May 14 2014, at 08:29 , Pavel Rappo <pavel.rappo at oracle.com> wrote:
>
> > Hi everyone,
> >
> > could you please review my change for JDK-7153400?
> >
> > http://cr.openjdk.java.net/~chegar/7153400/00/webrev/
> > http://ccc.us.oracle.com/7153400
> >
> > It's a long expected fix for a minor issue in the ThreadPoolExecutor.
> This has been agreed with Doug Lea. The exact same change (except for the
> test) is already in jsr166 repo:
> http://gee.cs.oswego.edu/cgi-bin/viewcvs.cgi/jsr166/src/main/java/util/concurrent/ThreadPoolExecutor.java?r1=1.151&r2=1.152
> >
> > Thanks
> > -Pavel
>
>