> On 9 Nov 2016, at 12:38, Yasumasa Suenaga <yasue...@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 >>>> >>> >>
