httpserver does not close connections when RejectedExecutionException occurs

[Chris Hegarty]

> On 9 Nov 2016, at 12:38, Yasumasa Suenaga <yasuenag at gmail.com> wrote:
> 
> Hi Yuji,
> 
>> http://cr.openjdk.java.net/~ykubota/8169358/webrev.01/

I think Yuji’s patch is good as is.

> I think you can use finally statement to close the connection in this case.

I don’t believe that this is possible. The handling of the connection may be
done in a separate thread, some time after execute() returns. I think it best
to just handle the specific case of REE, as it done in Yuji’s patch.

> Your patch cannot close the connection when any other runtime exceptions are occurred.
> 
> However, it is concerned double close if custom handler which is implemented by the user throws runtime exception after closing the connection.
> IMHO, you need to check the implementation of HTTP connection and dispatcher.

-Chris.

> 
> Thanks,
> 
> Yasumasa
> 
> 
> On 2016/11/08 18:53, KUBOTA Yuji wrote:
>> Hi Chris,
>> 
>> Thank you for your review and updating this issues on JBS.
>> 
>> I filed an issue:
>> https://bugs.openjdk.java.net/browse/JDK-8169358
>> 
>> I don't assign to me because I'm not a committer.
>> 
>> 2016-11-08 0:28 GMT+09:00 Chris Hegarty <chris.hegarty at oracle.com>:
>>>> * patch
>>>> diff --git
>>>> a/src/jdk.httpserver/share/classes/sun/net/httpserver/ServerImpl.java
>>>> b/src/jdk.httpserver/share/classes/sun/net/httpserver/ServerImpl.java
>>>> --- a/src/jdk.httpserver/share/classes/sun/net/httpserver/ServerImpl.java
>>>> +++ b/src/jdk.httpserver/share/classes/sun/net/httpserver/ServerImpl.java
>>>> @@ -442,10 +442,13 @@
>>>>                 logger.log (Level.TRACE, "Dispatcher (4)", e1);
>>>>                 closeConnection(conn);
>>>>             } catch (IOException e) {
>>>>                 logger.log (Level.TRACE, "Dispatcher (5)", e);
>>>>                 closeConnection(conn);
>>>> +            } catch (RejectedExecutionException e) {
>>>> +                logger.log (Level.TRACE, "Dispatcher (9)", e);
>>>> +                closeConnection(conn);
>>>>             }
>>>>         }
>>>>     }
>>>> _
>>>>     static boolean debug = ServerConfig.debugEnabled ();
>>> 
>>> 
>>> This looks ok. I wonder if some of these exceptions could be refactored
>>> into a catch clause with several exception types?
>> 
>> Yes, I agree to keep simple. I updated my patch as below.
>> http://cr.openjdk.java.net/~ykubota/8169358/webrev.01/
>> Could you review it again?
>> 
>> Thank you,
>> Yuji
>>> 
>>> -Chris.
>>> 
>>> 
>>> 
>>>> * steps to reproduce
>>>> 1. java -Djava.util.logging.config.file=logging.properties
>>>> SmallHttpServer
>>>> 2. post tcp connections by curl or other ways
>>>>     e.g.: while true; do curl -XPOST --noproxy 127.0.0.1
>>>> http://127.0.0.1:8080/; done
>>>> 3. wait RejectedExecutionException occurs as below and then
>>>> SmallHttpServer stops by this issue.
>>>> ----
>>>> Nov 05, 2016 12:01:48 PM sun.net.httpserver.ServerImpl$Dispatcher run
>>>> FINER: Dispatcher (7)
>>>> java.util.concurrent.RejectedExecutionException: Task
>>>> sun.net.httpserver.ServerImpl$Exchange at 37b50d9e rejected from
>>>> java.util.concurrent.ThreadPoolExecutor at 1b3178d4[Running, pool size =
>>>> 1, active threads = 0, queued tasks = 0, completed tasks = 7168]
>>>>        at
>>>> java.util.concurrent.ThreadPoolExecutor$AbortPolicy.rejectedExecution(java.base/ThreadPoolExecutor.java:2076)
>>>>        at
>>>> java.util.concurrent.ThreadPoolExecutor.reject(java.base/ThreadPoolExecutor.java:842)
>>>>        at
>>>> java.util.concurrent.ThreadPoolExecutor.execute(java.base/ThreadPoolExecutor.java:1388)
>>>>        at
>>>> sun.net.httpserver.ServerImpl$Dispatcher.handle(jdk.httpserver/ServerImpl.java:440)
>>>>        at
>>>> sun.net.httpserver.ServerImpl$Dispatcher.run(jdk.httpserver/ServerImpl.java:405)
>>>>        at java.lang.Thread.run(java.base/Thread.java:844)
>>>> (SmallHttpServer is stopping by not closing socket)
>>>> ----
>>>> 
>>>> *logging.properties
>>>> handlers = java.util.logging.ConsoleHandler
>>>> com.sun.net.httpserver.level = FINEST
>>>> java.util.logging.ConsoleHandler.level = FINEST
>>>> java.util.logging.ConsoleHandler.formatter =
>>>> java.util.logging.SimpleFormatter
>>>> 
>>>> * SmallHttpServer.java
>>>> import com.sun.net.httpserver.HttpExchange;
>>>> import com.sun.net.httpserver.HttpHandler;
>>>> import com.sun.net.httpserver.HttpServer;
>>>> 
>>>> import java.net.InetSocketAddress;
>>>> import java.util.Scanner;
>>>> import java.util.concurrent.LinkedBlockingQueue;
>>>> import java.util.concurrent.ThreadPoolExecutor;
>>>> import java.util.concurrent.TimeUnit;
>>>> 
>>>> public class SmallHttpServer {
>>>> 
>>>>    public static void main(String[] args) throws Exception {
>>>>        int POOL_SIZE = 1;
>>>>        String HOST = args.length < 1 ? "127.0.0.1" : args[0];
>>>>        int PORT = args.length < 2 ? 8080 : Integer.valueOf(args[1]);
>>>> 
>>>>        // Setup a minimum thread pool to rise
>>>> RejectExecutionException in httpserver
>>>>        ThreadPoolExecutor miniHttpPoolExecutor
>>>>                = new ThreadPoolExecutor(POOL_SIZE, POOL_SIZE, 0L,
>>>> TimeUnit.MICROSECONDS,
>>>>                        new LinkedBlockingQueue<>(1), (Runnable r) -> {
>>>>                            return new Thread(r);
>>>>                        });
>>>>        HttpServer httpServer = HttpServer.create(new
>>>> InetSocketAddress(HOST, PORT), 0);
>>>>        httpServer.setExecutor(miniHttpPoolExecutor);
>>>> 
>>>>        HttpHandler res200handler = (HttpExchange exchange) -> {
>>>>            exchange.sendResponseHeaders(200, 0);
>>>>            exchange.close();
>>>>        };
>>>>        httpServer.createContext("/", res200handler);
>>>> 
>>>>        httpServer.start();
>>>>        // Wait stdin to exit
>>>>        Scanner in = new Scanner(System.in);
>>>>        while (in.hasNext()) {
>>>>            System.out.println(in.nextLine());
>>>>        }
>>>>        httpServer.stop(0);
>>>>        miniHttpPoolExecutor.shutdownNow();
>>>>    }
>>>> }
>>>> 
>>>> 
>>>> Thanks,
>>>> Yuji
>>>> 
>>> 
>>