Re: httpserver does not close connections when RejectedExecutionException occurs

2016-11-09 Thread Yasumasa Suenaga

Hi Yuji,


http://cr.openjdk.java.net/~ykubota/8169358/webrev.01/


I think you can use finally statement to close the connection in this case.
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.


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 :

* 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);

Re: RFR(s): 8169196: [TESTBUG] Three tests from sun/net/www have undeclared dependencies

2016-11-09 Thread Daniel Fuchs

On 09/11/16 08:57, Sergei Kovalev wrote:

Hi Daniel,

Thank you for feedback.

Fixed and verified locally.
http://cr.openjdk.java.net/~skovalev/8169196/webrev.02/



Looks good - except for using a raw type at line 220:

220 Class ntlmProxyClass

should be:

Class ntlmProxyClass

No need for a new webrev.

best regards,

-- daniel