psiroky commented on code in PR #799:
URL: https://github.com/apache/maven-mvnd/pull/799#discussion_r1126459819
##########
daemon/src/main/java/org/mvndaemon/mvnd/daemon/Server.java:
##########
@@ -233,7 +233,16 @@ private void accept() {
try {
while (true) {
try (SocketChannel socket = this.socket.accept()) {
- client(socket);
+ try {
+ // execute the client connection handling inside a new
thread to guard against possible
+ // ThreadLocal memory leaks
+ // see https://github.com/apache/maven-mvnd/issues/798
for more details
+ Thread handler = new Thread(() -> client(socket));
+ handler.start();
+ handler.join();
+ } catch (Throwable t) {
+ LOGGER.error("Error handling a client connection", t);
+ }
Review Comment:
I think this technically changes the semantics a bit. Before, if the
`client(socket)` threw an exception, then the whole `while` loop would end,
which is not the case anymore. If there is a bug somewhere, then I guess the
previous version of the code could fail fast (leaving the while loop), while
the new would simply be waiting for another connection. Not sure how big of
problem this is.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscr...@maven.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
